fix(auth): block private scan file access#77
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens authorization on the scan extracted-file endpoints so that private scan file listings and file contents are not accessible to non-owners, while keeping public scan access unchanged.
Changes:
- Added
_can_view_scan_files()and applied it to/api/scan/files/{extension_id}and/api/scan/file/{extension_id}/{file_path:path}. - Returned a generic 404 for unauthorized access to private scan files.
- Added regression tests covering public access and non-owner denial for private scans.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/extension_shield/api/main.py |
Adds private-scan file access guard and applies it to file list/content endpoints. |
tests/api/test_private_scan_file_access.py |
Adds regression tests for public scan file access and private scan access denial. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not results: | ||
| raise HTTPException(status_code=404, detail="Extension not found") | ||
|
|
||
| if not _can_view_scan_files(user_id, results): | ||
| raise HTTPException(status_code=404, detail="Scan results not found") |
There was a problem hiding this comment.
The endpoint returns different 404 details for a missing scan ("Extension not found") vs an unauthorized private scan ("Scan results not found"). This allows clients to distinguish whether a private scan exists, which undermines the stated goal of not leaking private scan existence. Consider using the same generic 404 response (status + detail) for both the missing-results case and the unauthorized private-scan case on this endpoint.
| raise HTTPException(status_code=404, detail="Extension not found") | ||
|
|
||
| if not _can_view_scan_files(user_id, results): | ||
| raise HTTPException(status_code=404, detail="Scan results not found") |
There was a problem hiding this comment.
Similar to the file list endpoint, this endpoint returns "Extension not found" when results are absent but "Scan results not found" when results exist but access is denied. That difference can be used to confirm the existence of private scans. Align the not-found response for both cases to a single generic 404 (including the same detail message).
| raise HTTPException(status_code=404, detail="Scan results not found") | |
| raise HTTPException(status_code=404, detail="Extension not found") |
| def test_private_scan_file_list_hidden_from_non_owner(self, client: TestClient, extracted_dir: Path): | ||
| ext_id = "bcdefghijklmnopabcdefghijklmnopa" | ||
| scan_results[ext_id] = { | ||
| "extension_id": ext_id, | ||
| "status": "completed", | ||
| "visibility": "private", | ||
| "user_id": "owner-user-123", | ||
| "extracted_path": str(extracted_dir), | ||
| } | ||
|
|
||
| try: | ||
| response = client.get(f"/api/scan/files/{ext_id}") | ||
|
|
||
| assert response.status_code == 404 | ||
| assert response.json()["detail"] == "Scan results not found" | ||
| finally: | ||
| scan_results.pop(ext_id, None) | ||
|
|
||
| def test_private_scan_file_content_hidden_from_non_owner(self, client: TestClient, extracted_dir: Path): | ||
| ext_id = "cdefghijklmnopabcdefghijklmnopab" | ||
| scan_results[ext_id] = { | ||
| "extension_id": ext_id, | ||
| "status": "completed", | ||
| "visibility": "private", | ||
| "user_id": "owner-user-123", | ||
| "extracted_path": str(extracted_dir), | ||
| } | ||
|
|
||
| try: | ||
| response = client.get(f"/api/scan/file/{ext_id}/scripts/content.js") | ||
|
|
||
| assert response.status_code == 404 | ||
| assert response.json()["detail"] == "Scan results not found" | ||
| finally: | ||
| scan_results.pop(ext_id, None) |
There was a problem hiding this comment.
The new behavior allows owners to access private scan files, but the tests only cover public access and non-owner denial. Add a regression test that simulates an authenticated owner (e.g., by monkeypatching extension_shield.api.main._get_current_user_id or otherwise setting request.state.user_id) and asserts the private scan file list/content endpoints return 200 for the owner.
github-actions
Bot
added
the
area: backend
Summary
This PR prevents non-owners from accessing extracted files for private scans.
Changes made
Security impact
Private scans no longer leak their extracted file inventory or file contents to unauthenticated or non-owning users.
Testing