Skip to content

Best-practice: wildcard CORS w/ credentials + path-traversal defense-in-depth on /icons proxy #298

Description

@elfrost

Hi @msoedov,

While testing AI PatchLab (an open-source local-first SAST/SCA scanner) against a few real-world Python AI projects, I scanned agentic_security at `2aabcef` and wanted to flag two best-practice improvements. Filing as a single courtesy issue.

A separate finding was reported privately via GitHub Private Vulnerability Reporting (thanks for having that enabled). The full curated write-up — including the two items below, the FP analysis, and a placeholder for the privately-disclosed item — is at: https://elfrost.github.io/ai-patchlab/scans/msoedov-agentic-security.html

1. `agentic_security/middleware/cors.py:10` — wildcard CORS with credentials

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

The combination `allow_origins=["*"]` + `allow_credentials=True` is explicitly forbidden by the CORS spec (browsers will strip the credentials when they see `Access-Control-Allow-Origin: *` paired with `Access-Control-Allow-Credentials: true`), so this configuration doesn't behave the way the code reads. The intent is "any origin may make authenticated requests" but the actual behavior is "any origin may make unauthenticated requests, and credentials silently disappear cross-origin".

Two fixes depending on what you actually want:

  • If you need cross-origin credentialed access, enumerate explicit origins:
    ```python
    origins = ["http://localhost:3000", "https://your-frontend.example"]
    ```
  • If cross-origin credentialed access isn't needed, drop `allow_credentials=True` and the wildcard origins behave fine.

2. `agentic_security/routes/static.py:104` — path-traversal defense-in-depth on the `/icons/{icon_name}` proxy

```python
@router.get("/icons/{icon_name}")
async def serve_icon(icon_name: str) -> FileResponse:
icon_path = ICONS_DIR / icon_name
if not icon_path.exists():
url = f"https://registry.npmmirror.com/@lobehub/icons-static-png/latest/files/dark/{icon_name}\"
response = requests.get(url)
if response.status_code == 200:
icon_path.write_bytes(response.content)
...
```

Two interlocking concerns:

  1. `icon_name` from the path parameter is interpolated into `ICONS_DIR / icon_name` without validation. FastAPI's `{icon_name}` is single-segment by default, but `%2F`-encoded separators (and any future change to Starlette's URL handling) could allow `icon_name` values that resolve outside `ICONS_DIR`. Combined with `icon_path.write_bytes(...)`, that's an arbitrary-file-write surface.
  2. The same value is interpolated into the upstream `registry.npmmirror.com` URL. The host is fixed (so this isn't classic SSRF), but an attacker who can pick the path component can make the server cache an arbitrary file from `registry.npmmirror.com` under a name they control.

Recommended fix — strict allowlist on `icon_name` plus a containment check on the resolved path:

```python
import re
ICON_NAME_RE = re.compile(r"^[a-z0-9_-]+.png$")

@router.get("/icons/{icon_name}")
async def serve_icon(icon_name: str) -> FileResponse:
if not ICON_NAME_RE.match(icon_name):
raise HTTPException(status_code=400, detail="Invalid icon name")

icon_path = (ICONS_DIR / icon_name).resolve()
if not icon_path.is_relative_to(ICONS_DIR.resolve()):
    raise HTTPException(status_code=400, detail=\"Invalid icon name\")
...

```

(The same allowlist also protects against ever fetching from a weird URL path component upstream.)


Happy to open separate PRs for either of these. Thanks for agentic_security — beyond these two items and the one I reported privately, the scan turned up only false positives or out-of-scope items (test fixtures for your own PII detector, the vendored Vue.js, a jinja2 `Environment(autoescape=True)` that the scanner failed to read correctly).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions