fix(security): sandbox local media paths in cloud AI providers (rebase of #1216) - #1333
Conversation
All three cloud AI providers dispatch `analyze_image(image_url, ...)` on the
string's prefix: `s3://` and `http(s)://` are treated as remote sources, and
anything else fell through to an unguarded `open()`. A caller-supplied
absolute path, `../` traversal, or symlink could therefore read any file
readable by the service account.
The same unguarded sink existed in all three providers, not just the one named
in the issue:
- aws_rekognition.py `_prepare_image_input`
- azure_vision.py `_prepare_image_input`
- google_cloud.py inline `open()` in `analyze_image`
Introduce `cloud_ai/media_paths.py` as the single policy for local reads:
- Local reads are opt-in via `CLOUD_AI_MEDIA_ROOT`. Unset (the default)
disables them entirely, restricting providers to `s3://`/`https://`.
This is fail-closed, and answers the issue's open question.
- When a root is configured, both root and candidate are fully resolved
(`Path.resolve()` follows symlinks) and the candidate must be contained by
the root -- covering symlink escapes, not just lexical `..` segments.
- Non-regular files (FIFO, device, directory) are rejected, so a FIFO placed
inside the root cannot pin a `to_thread` worker forever.
- Rejection raises the new typed `UnsafeMediaPathError(CloudAIError)` instead
of silently returning empty bytes. Each provider re-raises `CloudAIError`
subclasses unchanged so the type survives to the caller.
- Providers read from the resolved path, not the caller string, narrowing the
check-to-open race.
- Error messages echo only the caller-supplied value; the resolved path is
logged server-side for forensics rather than returned.
Adds tests/unit/test_cloud_ai_media_paths.py (44 tests) covering absolute
paths, `../` traversal, symlink escape, non-regular files, the disabled
default, and per-provider propagation. Existing local-file tests now set
`CLOUD_AI_MEDIA_ROOT`. Full cloud AI suite: 505 passed.
Closes #1209
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… gaps (#1216) Addresses the three unresolved Copilot review threads on #1216: 1. Fail-closed on a misconfigured root. get_media_root() left resolve() non-strict, so CLOUD_AI_MEDIA_ROOT=/etc/passwd (a regular file) was accepted as the root; that file then passed its own is_relative_to() containment check and was returned as a permitted read. Require the resolved root to be an existing directory, raising ConfigurationError otherwise. This also surfaces a nonexistent-directory typo loudly instead of silently rejecting every candidate. 2. Cover the Google permitted-file branch. AWS/Azure verified successful reads but the Google class only had rejection cases, while the PR's coverage table claimed the check for all three providers. Add an end-to-end analyze_image test asserting the resolved file's bytes are assigned to vision.Image().content. 3. Correct the module docstring. Remote-scheme handling is provider- specific: only AWS Rekognition recognises s3:// (Azure and Google treat it as a local path, rejected while local reads are disabled), and all three accept plain http:// as well as https://. Focused suite: 47 passed (44 + 3 new). Full cloud AI provider suites: 368 passed. ruff/mypy clean; black formatted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFFcgtEzNyhrxJnHgimcd2
The event-loop offload tests from #1304/#1323 pass raw local paths to _prepare_image_input/analyze_image; with local reads now fail-closed behind CLOUD_AI_MEDIA_ROOT, they must opt in via tmp_path, matching the other pre-existing local-file tests. Generated with [Linear](https://linear.app/myxstack/issue/GRV-296/land-pr-1216-fixsecurity-sandbox-local-media-paths#agent-session-3138b916) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
🔍 PR Validation |
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Agent Completion Truth Gate: NOT_APPLICABLEEvidence agrees. Machine-readable verdict{
"details": {},
"reasons": [],
"verdict": "not_applicable"
} |
…ity-sandbox-local-media-paths-grv-296-e2e4
There was a problem hiding this comment.
Pull request overview
Hardens cloud AI image analysis by sandboxing local media reads.
Changes:
- Adds a shared, fail-closed path guard and typed errors.
- Applies validation while preserving worker-thread reads.
- Adds provider security tests and configuration guidance.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.env.example |
Documents the media sandbox setting. |
cloud_ai/__init__.py |
Exports sandbox APIs. |
cloud_ai/exceptions.py |
Adds UnsafeMediaPathError. |
cloud_ai/media_paths.py |
Implements path containment validation. |
providers/aws_rekognition.py |
Guards AWS local image reads. |
providers/azure_vision.py |
Guards Azure local image reads. |
providers/google_cloud.py |
Guards Google local image reads. |
test_aws_rekognition_provider.py |
Configures permitted roots in AWS tests. |
test_azure_vision_provider.py |
Configures permitted roots in Azure tests. |
test_google_cloud_provider.py |
Configures permitted roots in Google tests. |
test_cloud_ai_media_paths.py |
Adds comprehensive sandbox security tests. |
| # To enable local-path reads (dev/self-hosted only), set this to a directory | ||
| # that contains ONLY media you are willing to expose. Paths are fully resolved, | ||
| # so `../` traversal and symlinks that escape the root are rejected. | ||
| # Leave empty in production: use s3:// or https:// inputs instead. |
There was a problem hiding this comment.
Fixed in 5002553. .env.example now scopes the schemes per provider: http(s):// for all providers, s3:// for AWS Rekognition only, with an explicit note that Azure Vision and Google Vision treat s3:// as a local path (and therefore reject it while local reads are disabled). The production guidance line was updated to match.
Generated by Claude Code
| "Local media reads are disabled. Use an s3:// or https:// source, " | ||
| f"or set {MEDIA_ROOT_ENV_VAR} to the directory local media may be " | ||
| "read from.", |
There was a problem hiding this comment.
Fixed in 5002553. The disabled-reads UnsafeMediaPathError message no longer implies s3:// works everywhere — it now reads "https:// works for every provider; s3:// only for AWS Rekognition", so an Azure/Google caller is pointed at a scheme that actually resolves. Guard logic is unchanged; the existing message-substring assertions ("disabled", CLOUD_AI_MEDIA_ROOT) still hold.
Generated by Claude Code
The disabled-reads UnsafeMediaPathError message and .env.example both suggested s3:// as a recovery scheme for all three cloud AI providers, but only AWS Rekognition recognizes s3://. Azure Vision and Google Vision route s3:// through the disabled local-path branch, so following that guidance just raises UnsafeMediaPathError again. Reword both to scope s3:// to AWS Rekognition and point Azure/Google callers at https:// (valid for every provider). No logic change; the guard behavior is unchanged. Addresses the two Copilot review threads on this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8yJv2udCCnwN4u586e9PL
|
PR-remediation routine — status: Ran OBSERVE → conflict → CI gates on head 1. Governance gate red — duplicate canonical issue (your call).
→ Decision needed: close the superseded #1216, then set this body to 2. Two unresolved Copilot review threads — need a commit.
I can't push this fix — my branch guardrail restricts me to my own dev branch. Staged for whoever picks it up. 3. Merge is human-gated. CI note: at scan time Generated by Claude Code |
Canonical issue
Closes #1330. Supersedes #1216 (its two commits cherry-picked onto current
mainwith conflicts resolved), which was opened for the same work under #1209.Outcome
Local file reads in the three cloud AI providers'
analyze_imagepaths are now fail-closed: disabled unlessCLOUD_AI_MEDIA_ROOTis set, and when enabled every candidate path is resolved (symlinks followed) and must sit inside the resolved root. Escapes and non-regular files raise a typedUnsafeMediaPathError; a misconfigured root raisesConfigurationError. Full rationale, scoping notes, and acceptance-criteria mapping are in #1216.Scope
cloud_ai/media_paths.py, typed error,.env.exampledocs, 47 security tests, per-provider guard tests.mainhas since moved local file reads ontoasyncio.to_thread(perf: offload video-detail cache read, drop the stat probe #1304). The Azure and Google conflict resolutions keep both behaviors — validate viaresolve_local_media_path, then read the resolved path off the event loop (AWS already merged cleanly this way).mainby perf: offload video-detail cache read, drop the stat probe #1304/perf: shut down container services concurrently #1323 pass raw local paths, which the guard now rejects; four of them now opt in by settingCLOUD_AI_MEDIA_ROOT=tmp_path, matching the pre-existing local-file tests.92b980dbrings the branch up to date withmain(picks up the retired CI-investigator governance test rewrite that was failing CI on the previous head).analyze_videolocal-path handling (as in fix(security): sandbox local media paths in cloud AI providers (#1209) #1216).Risk
UnsafeMediaPathErroruntilCLOUD_AI_MEDIA_ROOTis set; no HTTP route reachesanalyze_imagetoday.Verification
On head of this branch (
92b980d):tests/unit/test_cloud_ai_media_paths.py+ AWS/Azure/Google provider suites + gh-aw governance suite: 369 passedruff checkclean on the new module and new test file (provider files retain pre-existing findings, untouched as in fix(security): sandbox local media paths in cloud AI providers (#1209) #1216)Production evidence
Not applicable — backend hardening with no user-facing surface; no route currently reaches
analyze_image(see #1216).Agent handoff