fix: wildcard CORS + credentials spec violation, path-traversal guard on /icons proxy

[DevamShah]

Closes #298

Summary

Remove the allow_credentials=True / allow_origins=["*"] spec-invalid combo and add a strict allowlist + path-containment check on the /icons/{icon_name} endpoint before any filesystem write or outbound fetch.

Problem / motivation

Two interlocking issues were filed in #298:

1. CORS (middleware/cors.py:10)

origins = ["*"]
app.add_middleware(CORSMiddleware, allow_origins=origins, allow_credentials=True, ...)

allow_origins=["*"] combined with allow_credentials=True is explicitly forbidden by the CORS spec (Fetch §4.7, RFC 6454 §7.2). Browsers respond by silently stripping credentials on every cross-origin request, so the configuration neither works as intended nor provides any actual access control. The code reads as "any origin may make authenticated requests" but behaves as "any origin may make unauthenticated requests." Because the app uses Bearer tokens in request headers (not cookies), allow_credentials is unnecessary here.

2. Path traversal / arbitrary-file-write surface (routes/static.py:100)

icon_path = ICONS_DIR / icon_name     # unsanitized path join
url = f"https://registry.npmmirror.com/.../dark/{icon_name}"  # unsanitized URL path
...
icon_path.write_bytes(response.content)   # writes attacker-controlled name

icon_name is taken directly from the URL path parameter and interpolated into both a filesystem path and an outbound URL with no validation. FastAPI's single-segment path parameters normally block literal /, but %2F-encoded slashes are handled inconsistently across Starlette versions and can decode to directory separators. The combined write surface represents CWE-22 (path traversal / arbitrary write) and a constrained SSRF-adjacent risk (controlled path component against a fixed host).

Change

agentic_security/middleware/cors.py

• Removed allow_credentials=True. Wildcard origins remain; credentialed cross-origin access was not required and the setting was non-functional.

agentic_security/routes/static.py

• Added module-level ICON_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+\.png$").
• serve_icon() now validates icon_name against ICON_NAME_RE before any I/O; returns HTTP 400 on mismatch.
• After path join, resolves the path and asserts is_relative_to(ICONS_DIR.resolve()) as a defense-in-depth containment check; returns HTTP 400 if outside the icons directory. This layer defends against any future URL-handling change that could pass through encoded separators.

tests/unit/test_cors_middleware.py (new)

• Asserts CORSMiddleware is registered.
• Asserts wildcard origins are not paired with allow_credentials=True.
• Asserts OPTIONS preflight returns CORS headers.
• Asserts Access-Control-Allow-Credentials: true is absent when ACAO: *.

tests/integration/routes/test_static_icon_validation.py (new)

• TestIconNameRegex: parametrized unit tests for 7 valid and 11 invalid name patterns including path traversal, encoded slashes, wrong extensions, null bytes, and double extensions.
• TestServeIconValidation: integration tests via TestClient confirming 400 for invalid names, pass-through to 404 (not 400) for a valid-but-absent icon, and 200 for a cached icon (mocked FS).

Security rationale

Finding	CWE	Impact
CORS wildcard + credentials	CWE-942 (Overly Permissive CORS)	Broken access control; credentials silently dropped — OWASP A01:2021
Icon path traversal + write	CWE-22 (Path Traversal)	Arbitrary write within the server's FS permissions — OWASP A03:2021
Icon URL path injection	CWE-73 (External Control of File Name or Path)	Controlled path against fixed upstream host

The regex allowlist follows the principle of positive validation (allowlist over denylist), which is the OWASP-recommended approach for filename inputs. The is_relative_to check is a belt-and-suspenders layer that survives any future URL-decoding changes upstream.

Testing / validation

python3 -m pytest tests/unit/test_cors_middleware.py \
                  tests/integration/routes/test_static_icon_validation.py \
                  tests/integration/routes/test_static.py -v

Result: 36 passed (new CORS + icon-validation tests plus the existing test_static.py suite), no regressions. icon_name is matched with re.fullmatch against an anchorless allowlist so a trailing newline (icon.png\n) is also rejected.

Regex positive/negative coverage verified independently:

Input	Expected	Result
openai.png	MATCH	MATCH
claude-3.png	MATCH	MATCH
../etc/passwd	NO MATCH	NO MATCH
../../secret.png	NO MATCH	NO MATCH
foo%2Fbar.png	NO MATCH	NO MATCH
icon.PNG	NO MATCH	NO MATCH
icon.png.sh	NO MATCH	NO MATCH
.png (empty stem)	NO MATCH	NO MATCH

[JackSpiece]

I reproduced the 36 focused tests, plus the full suite at 402 passed, 5 skipped, and 41 deselected. The icon allowlist and containment check look sound. One CORS concern remains: dropping allow_credentials fixes the invalid header combination, but allow_origins=["*"] still permits browser requests to unauthenticated state-changing endpoints such as /verify, /scan, and /stop when network policy allows them. The bundled UI derives SELF_URL from window.location.href, so it appears to be same-origin. Is cross-origin access required elsewhere? If not, removing CORS or using an explicit configurable allowlist would close the more important browser-to-local-service surface. Otherwise, I suggest tracking that separately and documenting the expected cross-origin client.

[msoedov]

@DevamShah @JackSpiece Thank you for the patch and testing