Feature/manual scan result detail#166
Conversation
du-dhartley
left a comment
There was a problem hiding this comment.
@AaronAlijani Pivoting away from the mostly duplicate manual table is good and this structure looks much better. A couple of things to introduce before we can safely commit this code though.
- It doesn't appear that there's any ownership enforcement on the endpoints - we should enforce that users can only retrieve and/or update their own scans and scan results. It's worth testing this and including a screenshot, then making the change and making the test again, showing that it no longer works.
- You've mentioned that no tests are required in the PR notes, endpoints that require authentication definitely do need tests wrapped around them to prevent the case I described in #1. We need tests that show a successful response when a record is pulled out / patched / deleted and so on, and the expected failure cases (duplicates, nonexistent records, etc)
- Is there a way for a user to remove a comment, or have we deliberately prevented this?
|
@AaronAlijani Did you come back to this PR after the feedback above? The 3 points above need to be addressed, though #3 is more a question at this stage |
|
Thanks David, I appreciate the feedback. I'll address all three: I'll add ownership checks to the GET, PATCH, and DELETE endpoints so users can only access their own manual verifications. Will include screenshots showing it blocks access to other users' records. |
|
I've addressed all three points: 1-Added ownership enforcement, a _check_ownership helper checks detail.user_id against current_user.id and returns 403 Forbidden if they don't match. Applied to GET, PATCH, and DELETE endpoints. |
|
@AaronAlijani Those ownership checks are looking good. The only one left to enforce is the and become the owner of that manual verification. We should ensure that the |
CI: Backend API
One or more checks failed. View logs |
|
Implemented the POST ownership validation for manual verification records. The endpoint now:
Also added and ran integration tests for:
All manual verification tests are passing locally. |
Summary
Add the manual_scan_result_detail table to store extra information for manually verified compliance controls. This table links back to scan_result with a one-to-one relationship, to keep scan_result as the single source of truth for all results (automated and manual) and use a detail table for manual-specific fields.
Update :
Add the manual_scan_result_detail table and full CRUD API for manually verified compliance controls. This table links back to scan_result with a one-to-one relationship, keeping scan_result as the single source of truth for all results (automated and manual) while using a detail table for manual-specific fields. Includes Pydantic schemas for request/response validation and API endpoints (POST, GET, PATCH, DELETE) registered in the application router.
Type of Change
Affected Components
/backend-api/frontend/engine(collectors / policies)/security/infrastructure/.github/workflows/docsMotivation
Replaces the previous manual_verification table (PR #148) based on team lead feedback. scan_result already holds scan_id, control_id, and status, so a separate table with the same columns creates duplication. This new design keeps scan_result as the single source for all results and adds a small detail table keyed to scan_result.id for manual-specific fields (user_id, comment).
Planner task: 25T3-RES-x-0xx — Build backend for manual control verification
Testing Done
Unit tests pass locally
Tested manually — describe how:
docker compose --profile all up --build -ddocker compose exec db psql -U autoaudit -d autoaudit -c "\d manual_scan_result_detail"docker compose psUpdate:
No tests required — explain why:
Security Considerations
No security impact. The table stores user_id as a foreign key reference and an optional text comment. No secrets, tokens, or sensitive data involved.
Breaking Changes
Rollback Plan
docker compose exec backend-api uv run alembic downgrade -1to drop the manual_scan_result_detail tableChecklist
Screenshots
Table verification in PostgreSQL:

Swagger docs screenshot: Prove all 5 endpoints are live and working in the running application.
