fix(audit-sprint-6c): compliance attestation UI (resubmit, was #173) - #185
Merged
Merged
Conversation
…tors
The 2026-05-23 audit deep-dive caught that ~157 control evaluators
across SOC2/ISO27001/HIPAA/PCI-DSS/CIS/NIST-CSF/GDPR packs emit
`status='not_evaluated'` with a gap of `code='manual_attestation_required'`
— but there was no UI for a human to complete the attestation. The
framework packs were structurally complete but practically PowerPoint.
This PR closes the loop end-to-end with a two-state workflow
(claimed → reviewed) enforcing separation of duties.
Deliverables (1337 net lines)
1. Migration `20260624021_audit_sprint6c_control_attestations.sql`:
- org_control_attestations table (org_id + framework_id + control_key,
status, claimed_by/at, reviewed_by/at, rejected_reason, evidence_id
NOT NULL, notes)
- CHECK constraints:
* status IN ('claimed','reviewed','rejected')
* reviewed_by != claimed_by when status='reviewed' (segregation
of duties — DB-enforced, defense in depth alongside the
server-action check)
* rejected requires non-empty rejected_reason
- RLS policies: org-scoped SELECT/INSERT/UPDATE via org_members
- updated_at trigger
- Compound indexes for the list-resolver join + a partial index on
pending ('claimed') rows for the "awaiting review" bucket
2. lib/compliance/attestations.ts — data layer
- listControlsNeedingAttestation(orgId): joins
org_control_evaluations rows whose details.gaps contains
MANUAL_GAP_CODE against the latest attestation per
(framework, control). Returns ControlNeedingAttestation[] the
UI groups into 3 buckets.
- insertAttestationClaim() + updateAttestationReview() — narrow
writers with their own validation (separation of duties, reject
reason required, status check).
3. app/app/actions/compliance-attestations.ts — server actions
- claimAttestation, reviewAttestation, listMyAttestations
- Each wraps the data layer with getUserOrgMembership() auth + a
writeAuditLog() hash-chain entry (Sprint 1's audit-engine).
- Hash-chain failure is non-blocking (logged) so an unrelated
chain conflict can't break the workflow.
- TODO marker: once Sprint 1 PR #162 merges, wrap with
assertOrgCanWrite() from lib/billing/enforce-grace-period.ts
so past-due orgs in read-only mode can't attest.
4. /app/compliance/attestations page + AttestationsClient
- 3 tabs: Awaiting attestation / Awaiting review / Reviewed
- Card per control with framework chip + control key + message +
conditional action button (Claim / Review / "Awaiting another
reviewer" if you're the claimer)
- ClaimDialog: evidence-id input + notes textarea + Cancel/Claim
- ReviewDialog: evidence + claimer-notes + Approve/Reject + the
segregation-of-duties banner when reviewer == claimer
- RejectDialog (AlertDialog): mandatory reason textarea
- Built on Sprint 4c Dialog/AlertDialog/Toaster primitives.
- Empty states for each bucket.
5. __tests__/lib/compliance/attestations.test.ts (7 tests)
- List filters out non-manual_attestation_required gaps
- List attaches latest attestation per (fw, control)
- List returns [] when no matching evaluations
- updateAttestationReview throws on reject-without-reason
- updateAttestationReview throws on reviewer == claimer
- updateAttestationReview throws when status != 'claimed'
- approve path returns the updated row
Built on top of fix/audit-sprint-4c-modal-primitives (PR #167) for the
Dialog/AlertDialog/Toaster primitives.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: 5326/5341 pass (+7 new attestation tests on top of 4c's 5319)
Test plan after merge
- Apply migration 20260624021 to staging.
- Visit /app/compliance/attestations as a user whose org has at
least one active framework with manual evaluators.
- As user A, claim an attestation with an evidence row id.
- As user B (different user), open the same row, click Approve.
- Verify writeAuditLog wrote two hash-chained entries:
compliance.attestation.claimed (user A)
compliance.attestation.approved (user B)
- Try to approve your own claim — UI should show the segregation
banner and disable the Approve button; if you bypass the client
check, the server action / DB CHECK both reject.
- Reject with no reason — UI Submit stays disabled; server action
rejects if you bypass it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR implements a complete manual attestations workflow for compliance controls: a Supabase table with status transitions and RLS, data-access functions to query controls and mutate attestation claims/reviews, server actions for authorization and audit logging, a server page for initial state fetch, and a React UI with tabbed control cards and dialogs for claiming and reviewing attestations. ChangesManual Attestations Workflow
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as page.tsx
participant ListAction as listMyAttestations()
participant DB as Database
participant Client as AttestationsClient
User->>Page: navigate to /app/compliance/attestations
Page->>Page: load system state & current user
Page->>ListAction: call (server action)
ListAction->>DB: listControlsNeedingAttestation(orgId)
DB-->>ListAction: ControlNeedingAttestation[]
ListAction-->>Page: actionOk(controls)
Page->>Client: render with currentUserId + controls
Client->>Client: group by latestAttestation.status<br/>into buckets
Client-->>User: render tabs + control cards
sequenceDiagram
participant User
participant Dialog as ClaimDialog<br/>ReviewDialog<br/>RejectDialog
participant Action as claimAttestation()<br/>reviewAttestation()
participant DataAccess as insertAttestationClaim()<br/>updateAttestationReview()
participant Audit as writeAuditLog() [optional]
rect rgba(200, 100, 100, 0.5)
Note over User,Audit: Claim Flow
User->>Dialog: submit evidenceId + notes
Dialog->>Dialog: validate evidence
Dialog->>Action: invoke server action
Action->>DataAccess: insert claim
DataAccess-->>Action: AttestationRow
Action->>Audit: attempt audit log (non-blocking)
Audit-->>Action: result (ignored on error)
Action-->>Dialog: actionOk(AttestationRow)
Dialog->>Dialog: show success toast + close
end
rect rgba(100, 100, 200, 0.5)
Note over User,Audit: Review/Reject Flow
User->>Dialog: click approve or enter reject reason
Dialog->>Dialog: validate (SoD warning,<br/>reason required for reject)
Dialog->>Action: invoke with<br/>decision + rejectedReason
Action->>DataAccess: update review
DataAccess-->>Action: AttestationRow
Action->>Audit: attempt audit log (non-blocking)
Audit-->>Action: result (ignored on error)
Action-->>Dialog: actionOk(AttestationRow)
Dialog->>Dialog: show success toast + close
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 348980ec9d
ℹ️ 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".
Comment on lines
+226
to
+227
| .eq('id', input.attestationId) | ||
| .maybeSingle(); |
There was a problem hiding this comment.
updateAttestationReview reads and updates by id only while using createSupabaseAdminClient (service-role), so it bypasses RLS and does not prove the attestation belongs to the caller’s org. If a user obtains another org’s attestation UUID, they can approve/reject that row, and reviewAttestation will also write an audit log under the reviewer’s org, producing cross-tenant state corruption and misleading audit history.
Useful? React with 👍 / 👎.
| control_key: input.controlKey, | ||
| status: 'claimed', | ||
| claimed_by: input.claimedBy, | ||
| evidence_id: input.evidenceId, |
There was a problem hiding this comment.
The claim path inserts evidence_id without checking that the referenced evidence row belongs to input.orgId. Because this write also uses the admin client and the FK only checks existence of org_evidence.id, a caller who knows a foreign evidence UUID can create an attestation in their org that points to another org’s evidence, breaking tenant boundaries and data integrity.
Useful? React with 👍 / 👎.
♿ Accessibility Test ResultsTests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resubmit after base 4c branch deletion. Rebased onto main.
Summary by CodeRabbit
Release Notes