chore(readiness): assert query-hash HMAC guard is active (PIA-2)#532
Merged
Conversation
Close the operational half of PIA-2 by making `npm run check:production-readiness` positively assert the query-hash redaction guard, instead of the prior silent no-op that only fired inside a production-like environment. - Add checkQueryHashGuardWiring(): reads src/instrumentation.ts and asserts it still invokes requireQueryHashSecret(), so the fail-closed boot guard can't be silently dropped by a refactor. Emits an observable PASS in every environment, including CI (where the secret-presence check stays intentionally quiet). - Exercise the real requireQueryHashSecret() guard rather than re-encoding the env rule (mirrors requireServerEnv/requireOpenAIEnv): PASS when the secret is set; fail closed only in a production-like env. - Correct stale PIA-2 docs (privacy-impact-assessment.md §5.1/§10 + summary) that still claimed nothing enforced the secret in production. The code guard, boot wiring, unit tests, and GitHub secret already shipped; the remaining PIA-2 step is the operator placing RAG_QUERY_HASH_SECRET in the deploy host's secret store. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
📝 WalkthroughWalkthroughProduction-readiness checks now verify query-hash secret guard wiring and runtime behavior. The privacy impact assessment updates PIA-2 documentation to describe production fail-closed enforcement and the remaining operator configuration action. ChangesQuery-hash enforcement
Sequence Diagram(s)sequenceDiagram
participant ProductionReadiness as production-readiness.ts
participant Instrumentation as src/instrumentation.ts
participant Environment as envModule
ProductionReadiness->>Instrumentation: verify requireQueryHashSecret wiring
ProductionReadiness->>Environment: call requireQueryHashSecret()
Environment-->>ProductionReadiness: pass or throw
Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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: 06dc4be72b
ℹ️ 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".
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
…c-secret-0e44d3 # Conflicts: # docs/privacy-impact-assessment.md
Contributor
Merge conflicts resolved in commit
|
Contributor
Branch is now up to date with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/privacy-impact-assessment.md`:
- Around line 211-221: Update the documentation around the PIA-2 status and the
additionally referenced sections to match the actual behavior of
requireQueryHashSecret() and its direct invocation from instrumentation: state
that the guard throws whenever RAG_QUERY_HASH_SECRET is absent, without claiming
it is limited to NODE_ENV=production or that dev/CI retain the legacy digest.
Preserve the documented operator action and readiness-check coverage.
In `@scripts/production-readiness.ts`:
- Around line 232-246: The readiness check around requireQueryHashSecret must
not suppress failures outside production. Reuse the same production-like
environment predicate as the boot guard, or explicitly add a diagnostic to
result when the guard throws in dev/CI, so readiness cannot report READY while
the direct guard would fail; preserve the existing production failure reporting.
- Around line 139-166: Update checkQueryHashGuardWiring so it verifies an
executable requireQueryHashSecret call in the instrumentation startup path
rather than matching raw source text that may appear in comments, strings, or
declarations. Use an AST-based check or equivalent syntax-aware parsing, while
preserving the existing pass/failure reporting and unreadable-file handling.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5930f116-4e5c-4da5-b41d-5c2ca8ffeb8d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
docs/privacy-impact-assessment.mdscripts/production-readiness.ts
BigSimmo
enabled auto-merge (squash)
July 12, 2026 18:44
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
Copilot AI
added a commit
that referenced
this pull request
Jul 12, 2026
Combine updated PIA-2 HMAC wording from main (#532) with this branch's scoped PIA-3 closure wording (durable rag_queries.answer log rather than answer text broadly).
1 task
BigSimmo
added a commit
that referenced
this pull request
Jul 12, 2026
…log (#535) * docs(privacy): scope PIA-3 closure to the durable rag_queries.answer log Review fix: the earlier PIA-3 wording overclaimed that generated answer text is "dropped at rest by default." The durable rag_queries.answer log is gated by RAG_PERSIST_ANSWER_TEXT, but rag_response_cache.payload still holds the full answer (owner-scoped, ~5-min TTL). Tighten the register/APP-11/recommendation wording to the durable log and add an explicit "Residual (scoped out)" note for the short-lived response cache, which is intentionally not gated (nulling its payload would defeat caching). No behaviour change; documentation accuracy only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * fix: resolve merge conflict in docs/privacy-impact-assessment.md Combine updated PIA-2 HMAC wording from main (#532) with this branch's scoped PIA-3 closure wording (durable rag_queries.answer log rather than answer text broadly). * docs(privacy): correct cache-retention wording + finish APP 1/5 reconcile Resolves the leftover merge-conflict markers in §11 (kept main's PIA-2 "enforced in code" language alongside the PIA-3 durable-log scoping). Codex (P2): rag_response_cache TTL gates reads only — sharedCacheSelector filters on expires_at, replaceSharedCacheRow overwrites only the same key, and there is no purge cron (per §8). Reword the residual/data-table/APP-11/Overall entries so they no longer describe a read-TTL as a retention bound; the row persists at rest until a same-key overwrite or manual flush. CodeRabbit (Major): finish the APP 1/5 reconciliation the autofix started — align the §1 register (PIA-1, PIA-5) and the PIA-5 detail (add a Progress note) with the §9 matrix, so the whole doc reflects the live privacy page + composer notice (PR #513). Residual is the APP 8 DPA/ZDR basis plus broader data-handling docs. Docs only; no behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
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
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.
Summary
Closes the operational (🤖) half of PIA-2 — mandatory query-hash HMAC secret: make
npm run check:production-readinesspositively assert that the query-hash redaction guard is active, instead of the prior check that was a silent no-op unless the script itself ran inside a production-like environment.Without
RAG_QUERY_HASH_SECRET, logged clinical-query hashes degrade to unsalted, dictionary-reversible SHA-256 — defeating query redaction. The fail-closed boot guard (requireQueryHashSecret()insrc/instrumentation.ts), its unit tests, and the GitHub secret already shipped; this PR adds the readiness assertion and corrects stale docs.Changes
scripts/production-readiness.tscheckQueryHashGuardWiring()(new): readssrc/instrumentation.tsand asserts it still invokesrequireQueryHashSecret(), so the fail-closed call can't be silently dropped by a refactor. Emits an observable PASS in every environment, including CI.requireQueryHashSecret()guard instead of duplicating!env.RAG_QUERY_HASH_SECRET(mirrors the existingrequireServerEnv/requireOpenAIEnvcalls): PASS when the secret is set; fails closed only in a production-like env.docs/privacy-impact-assessment.md: correct §5.1/§10 + summary table, which still claimed "nothing enforces it in production" (stale since the guard shipped).Behaviour by environment
check:production-readiness:ci, no secret, not prod-like)NODE_ENV/VERCEL_ENV=production), no secretRemaining (out of scope for this PR)
🧑 Operator: place
RAG_QUERY_HASH_SECRETin the deploy host's secret store on the live project. The code enforces its presence but cannot supply the value.Verification
npm run verify:pr-localnpm run check:production-readiness— all three branches demonstrated (wiring PASS everywhere; secret-set PASS; production-like fail-closed exit 1)tests/instrumentation.test.ts+tests/env-query-hash-secret.test.ts(10/10)typecheck/eslint/prettiercleanClinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy) — unchangedenv.tsstaysserver-only, the readiness script readsinstrumentation.tsstaticallyNotes
🤖 Generated with Claude Code