mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix(security): comprehensive security audit and hardening
Auth: login rate limit 30/min (was 500), global rate limit 1000/min (was unlimited), password/username max lengths on all Zod schemas, session invalidation on role change, API key legacy scan bounded to 100 keys. SVG: hardened regex sanitizer with CDATA stripping, XML entity decoding, set/animate/iframe/embed blocking, comprehensive data: URI blocking, use element external href blocking. 11 attack payload fixtures added. SSRF: fixed DNS rebinding TOCTOU by pinning resolved IPs via custom HTTP/HTTPS agents. Added 6to4 and NAT64 to blocked IPv6 ranges. Docker: capability dropping (cap_drop ALL + minimal cap_add), resource limits (4g/8g mem, 512/1024 pids), healthcheck timeout, password removed from startup banner, default password warning comments. Network: CSP and HSTS applied in all environments (not just production), stack traces removed from all error responses, internal paths stripped from error details, per-route rate limits on uploads (60/min) and URL fetches (200/hour). Files: exclusive temp file creation (O_EXCL), disk space circuit breaker, per-user storage quotas, settings payload 64KB size guard. Python sidecar: script name allowlist in dispatcher, minimal environment for subprocess spawns. Dependencies: fixed 6 production CVEs (drizzle-orm, fastify, fast-uri, @fastify/static, next, archiver/lodash). Pinned all GitHub Actions to SHA hashes. 114 security tests added. Full OWASP Top 10 penetration test matrix verified against production Docker container (30/30 pass after hardening).
This commit is contained in:
@@ -203,11 +203,10 @@ describe("buildTagArgs", () => {
|
||||
expect(args).toContain("-Copyright=");
|
||||
});
|
||||
|
||||
it("filters unsafe field names from removal", () => {
|
||||
const args = buildTagArgs({ fieldsToRemove: ["Artist", "rm -rf /", "../../etc"] });
|
||||
expect(args).toContain("-Artist=");
|
||||
expect(args).not.toContain("-rm -rf /=");
|
||||
expect(args).not.toContain("-../../etc=");
|
||||
it("rejects unsafe field names from removal", () => {
|
||||
expect(() => buildTagArgs({ fieldsToRemove: ["Artist", "rm -rf /", "../../etc"] })).toThrow(
|
||||
"Invalid tag name",
|
||||
);
|
||||
});
|
||||
|
||||
it("allows field names with colons and hyphens", () => {
|
||||
|
||||
+22
-19
@@ -3,13 +3,14 @@ import { MAX_REDIRECTS, safeFetch, validateFetchUrl } from "../../../apps/api/sr
|
||||
|
||||
describe("validateFetchUrl", () => {
|
||||
it("allows valid public HTTP URL", async () => {
|
||||
await expect(
|
||||
validateFetchUrl("https://images.unsplash.com/photo.jpg"),
|
||||
).resolves.toBeUndefined();
|
||||
const result = await validateFetchUrl("https://images.unsplash.com/photo.jpg");
|
||||
expect(result).toHaveProperty("resolvedIp");
|
||||
expect(typeof result.resolvedIp).toBe("string");
|
||||
});
|
||||
|
||||
it("allows valid public HTTP URL without TLS", async () => {
|
||||
await expect(validateFetchUrl("http://example.com/image.png")).resolves.toBeUndefined();
|
||||
const result = await validateFetchUrl("http://example.com/image.png");
|
||||
expect(result).toHaveProperty("resolvedIp");
|
||||
});
|
||||
|
||||
it("rejects non-HTTP schemes", async () => {
|
||||
@@ -68,10 +69,11 @@ describe("validateFetchUrl", () => {
|
||||
await expect(validateFetchUrl("http://[2001:DB8::1]/image.jpg")).rejects.toThrow("private");
|
||||
});
|
||||
|
||||
it("allows a public IP address directly in URL", async () => {
|
||||
it("allows a public IP address directly in URL and returns resolved IP", async () => {
|
||||
// Exercises the early-return path in resolveAndCheck when hostname is a
|
||||
// non-private IP literal (covers the `return` after the isIP check).
|
||||
await expect(validateFetchUrl("http://8.8.8.8/image.jpg")).resolves.toBeUndefined();
|
||||
const result = await validateFetchUrl("http://8.8.8.8/image.jpg");
|
||||
expect(result).toEqual({ resolvedIp: "8.8.8.8" });
|
||||
});
|
||||
|
||||
it("rejects invalid URLs", async () => {
|
||||
@@ -124,15 +126,14 @@ describe("validateFetchUrl with DNS mocking", () => {
|
||||
});
|
||||
|
||||
it("handles DNS lookup returning a single result object", async () => {
|
||||
// Covers the Array.isArray fallback branch (line 45: wrapping non-array in [])
|
||||
// Covers the Array.isArray fallback branch (wrapping non-array in [])
|
||||
const dns = await import("node:dns/promises");
|
||||
vi.mocked(dns.lookup).mockResolvedValueOnce({
|
||||
address: "203.0.113.1",
|
||||
family: 4,
|
||||
} as never);
|
||||
await expect(
|
||||
validateFetchUrl("http://single-result.example.com/image.jpg"),
|
||||
).resolves.toBeUndefined();
|
||||
const result = await validateFetchUrl("http://single-result.example.com/image.jpg");
|
||||
expect(result).toEqual({ resolvedIp: "203.0.113.1" });
|
||||
});
|
||||
|
||||
it("rejects when DNS returns multiple addresses with one private", async () => {
|
||||
@@ -163,9 +164,11 @@ describe("safeFetch", () => {
|
||||
} as unknown as Response;
|
||||
}
|
||||
|
||||
// HTTP URLs use global fetch (pinned via IP replacement); HTTPS uses node:https
|
||||
// with a pinned agent. These tests exercise the HTTP path via the mocked fetch.
|
||||
it("returns response for a direct (non-redirect) fetch", async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(200));
|
||||
const res = await safeFetch("https://example.com/image.jpg");
|
||||
const res = await safeFetch("http://93.184.216.34/image.jpg");
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -173,12 +176,12 @@ describe("safeFetch", () => {
|
||||
it("follows a redirect chain within MAX_REDIRECTS", async () => {
|
||||
// 3 redirects then a 200
|
||||
mockFetch
|
||||
.mockResolvedValueOnce(mockResponse(302, { location: "https://example.com/hop1" }))
|
||||
.mockResolvedValueOnce(mockResponse(301, { location: "https://example.com/hop2" }))
|
||||
.mockResolvedValueOnce(mockResponse(307, { location: "https://example.com/final" }))
|
||||
.mockResolvedValueOnce(mockResponse(302, { location: "http://93.184.216.34/hop1" }))
|
||||
.mockResolvedValueOnce(mockResponse(301, { location: "http://93.184.216.34/hop2" }))
|
||||
.mockResolvedValueOnce(mockResponse(307, { location: "http://93.184.216.34/final" }))
|
||||
.mockResolvedValueOnce(mockResponse(200));
|
||||
|
||||
const res = await safeFetch("https://example.com/start");
|
||||
const res = await safeFetch("http://93.184.216.34/start");
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
@@ -187,23 +190,23 @@ describe("safeFetch", () => {
|
||||
// Return redirects for every call (MAX_REDIRECTS + 1 iterations, all redirects)
|
||||
for (let i = 0; i <= MAX_REDIRECTS; i++) {
|
||||
mockFetch.mockResolvedValueOnce(
|
||||
mockResponse(302, { location: `https://example.com/hop${i + 1}` }),
|
||||
mockResponse(302, { location: `http://93.184.216.34/hop${i + 1}` }),
|
||||
);
|
||||
}
|
||||
|
||||
await expect(safeFetch("https://example.com/start")).rejects.toThrow("Too many redirects");
|
||||
await expect(safeFetch("http://93.184.216.34/start")).rejects.toThrow("Too many redirects");
|
||||
});
|
||||
|
||||
it("rejects a redirect to a private IP", async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(302, { location: "http://127.0.0.1/evil" }));
|
||||
|
||||
await expect(safeFetch("https://example.com/image.jpg")).rejects.toThrow("private");
|
||||
await expect(safeFetch("http://93.184.216.34/image.jpg")).rejects.toThrow("private");
|
||||
});
|
||||
|
||||
it("throws when redirect has no Location header", async () => {
|
||||
mockFetch.mockResolvedValueOnce(mockResponse(302));
|
||||
|
||||
await expect(safeFetch("https://example.com/image.jpg")).rejects.toThrow(
|
||||
await expect(safeFetch("http://93.184.216.34/image.jpg")).rejects.toThrow(
|
||||
"Redirect without Location header",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -83,6 +83,7 @@ vi.mock("../../../apps/api/src/lib/feature-status.js", () => ({
|
||||
|
||||
vi.mock("../../../apps/api/src/lib/errors.js", () => ({
|
||||
formatZodErrors: (issues: Array<{ message: string }>) => issues.map((i) => i.message).join("; "),
|
||||
stripInternalPaths: (msg: string) => msg,
|
||||
}));
|
||||
|
||||
vi.mock("sharp", () => ({
|
||||
|
||||
Reference in New Issue
Block a user