SECURITY-169-API-40: harden media upload and variant filename privacy#2925
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1592fb561
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| $slug = Str::slug((string) $asset->asset_key) ?: 'asset'; | ||
|
|
||
| return sprintf('org-%d/media-asset-%d-%s', (int) $asset->org_id, (int) $asset->getKey(), $slug); |
There was a problem hiding this comment.
When OSS sync is enabled, generated variant paths are copied verbatim by MediaAssetStorageSyncService::targetPath(), so this new org-%d/media-asset-%d-%s directory makes the upload in backend/tests/Feature/V0_5/MediaLibraryOssSyncTest.php:62-66 land under storage/media-library/variants/org-0/media-asset-<id>-pr-media-01-cover/... while that test still asserts the old storage/media-library/variants/pr-media-01-cover/... keys. Since backend/phpunit.xml includes tests/Feature, the broader PHPUnit suite will fail until those expectations, or a compatibility path, are updated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6a8aacbf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "status": "pass" | ||
| }, | ||
| "scope_validation": { | ||
| "evidence": "All 6 changed paths are listed in SECURITY-169-API-40 allowed_paths; route listing, fresh SQLite migration, focused tests, Pint, YAML/JSON parse, and git diff --check passed.", |
There was a problem hiding this comment.
When this PR is processed as a PR-train item, this pass is inaccurate: git diff --name-only dc22b30^ dc22b30 includes backend/tests/Unit/Services/BigFive/ResultPageV2/BigFiveResultPageV2CoreBodyPreviewTest.php, but the SECURITY-169-API-40 allowed_paths list does not include that file. Because backend/AGENTS.md requires stopping on out-of-scope drift, the ledger now tells operators the scope gate passed even though the manifest would reject one changed file.
Useful? React with 👍 / 👎.
fermatmind
deleted the
codex/security-169-api-40-harden-media-upload-and-variant-filename-privacy
branch
1 participant
What changed
Why
Close SECURITY-169 API40 findings for SEO image variant overwrite and original filename disclosure.
Validation
php artisan route:list --no-ansi(237 lines)git diff --check, and scope validation passedci_verify_mbti.sh: 1182 passed / 150106 assertions; same 4 inherited Big Five rendered-QA failures recorded by API39Deferred items
source_original_namevalues are filtered from API output; no production data mutation is performed here.