docs: close out hook-hardening (#280) - #327
Conversation
…d trust-boundary notes - docs/hook_hardening.md: status table updated to reflect that mitigation 3 (per-turn audit log, hook_audit.jsonl) shipped via #314. Issue #280 is closed by this PR; the remaining suggestions in the original report (model-side reference-existence check, hook-side allowlist for belief shapes) remain declined per the spec's own reasoning, with the model-behavior side captured in the docs noted below. - docs/PHILOSOPHY.md: new 'Trust boundary at the hook surface' section per hook_hardening.md lines 273-276 (the spec's implementation tracker called for it). Frames what the hook layer can and cannot enforce. - docs/LIMITATIONS.md: residual-risk Sharp-edges entry — model-layer compliance with the framing tag is not enforceable from Python; the audit log is the recovery surface.
Reviewer's GuideDocuments the completion of the hook-hardening campaign by marking all ratified mitigations as shipped, explicitly recording the declined mitigations and their rationale, adding a philosophy section that defines the trust boundary at the hook surface, and capturing the remaining model-layer risk in the limitations doc. Sequence diagram for per-turn hook processing and audit loggingsequenceDiagram
actor User
participant CLI as Aelfrice_CLI
participant Hook as UserPromptSubmit_hook
participant DB as Belief_Store_SQLite
participant Model as LLM_Model
participant Audit as Hook_Audit_Log
User ->> CLI: Submit_prompt(text)
CLI ->> Hook: OnUserPromptSubmit(text)
Hook ->> DB: Retrieve_relevant_beliefs(text)
DB -->> Hook: beliefs
Hook ->> Hook: Escape_tag_substrings_in_beliefs()
Hook ->> Hook: Format_belief_block_with_framing_tag()
Hook ->> Model: Send(user_text, framed_belief_block)
Model -->> CLI: Response
Hook ->> Audit: Write_record(turn_id, user_text, framed_belief_block)
Flow diagram for hook-hardening mitigations and decisionsflowchart TD
Start["Original_issue_280: four_suggested_mitigations"]
Start --> M1["Mitigation_1:\nFraming_tag_contract"]
Start --> M2["Mitigation_2:\nRender_time_escape"]
Start --> M3["Mitigation_3:\nPer_turn_audit_log"]
Start --> M4["Mitigation_4:\nReference_existence_check_in_model"]
Start --> M5["Mitigation_5:\nHook_side_allowlist_for_belief_shapes"]
M1 -->|"Ratified_in_spec_memo"| M1_impl["Implemented_in_hook_layer"]
M2 -->|"Ratified_in_spec_memo"| M2_impl["Implemented_in_hook_layer"]
M3 -->|"Ratified_in_spec_memo"| M3_impl["Implemented_in_hook_layer\n+ audit_log_config"]
M4 -->|"Declined:\nmodel_behavior_contract"| M4_doc["Documented_as_model_layer_duty\nin PHILOSOPHY_and_LIMITATIONS"]
M5 -->|"Declined:\nwould_block_legitimate_beliefs"| M5_doc["Documented_as_foot_gun;\nrender_time_escape_used_instead"]
M1_impl --> Status["Hook_hardening_status:\nAll_ratified_mitigations_shipped"]
M2_impl --> Status
M3_impl --> Status
M4_doc --> Residual["Residual_risk_is_model_layer"]
M5_doc --> Residual
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
hook_hardening.md, consider avoiding hard-coded line number references (e.g., “lines 240–252” and “lines 273–276”), since they will drift as the document evolves and become misleading over time. - In the status section of
hook_hardening.md, you may want to rephrase time-relative language like “#280 closes on this PR” to something PR-agnostic so the docs read cleanly outside the context of this specific change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `hook_hardening.md`, consider avoiding hard-coded line number references (e.g., “lines 240–252” and “lines 273–276”), since they will drift as the document evolves and become misleading over time.
- In the status section of `hook_hardening.md`, you may want to rephrase time-relative language like “#280 closes on this PR” to something PR-agnostic so the docs read cleanly outside the context of this specific change.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[claim:review:Setr:2026-04-30T22:22:58Z] |
|
[release:review:Setr:2026-04-30T22:24:08Z] |
Summary
Closes #280. Three docs-only changes that close out the hook-hardening campaign:
docs/hook_hardening.md— status table updated. Mitigation 3 (per-turn audit log) shipped via feat: per-turn hook audit log (#280 mitigation 3) #314; that row was still showing "Outstanding". The closing paragraph also explicitly records that the two remaining suggestions from the original hook hardening: UserPromptSubmit memory hook is an injection surface #280 body (model-side reference-existence check, hook-side allowlist) are declined per this same memo's reasoning.docs/PHILOSOPHY.md— new section Trust boundary at the hook surface. The spec's implementation tracker (docs/hook_hardening.mdlines 273-276) called for this paragraph alongside the residual-risk note. Frames what the hook layer can and cannot enforce so future readers don't re-litigate the declined mitigations.docs/LIMITATIONS.md— Sharp-edges entry covering the residual model-layer risk: hook framing relies on the model honoring the trust boundary; the audit log is the recovery surface.Why this shape
The original #280 report listed four suggested mitigations. The ratified spec memo (
docs/hook_hardening.md, landed in #292) accepted three (framing-tag contract, render-time tag-substring escape, per-turn audit log) and explicitly declined two:aelfrice.belief.contentis arbitrary user / ingest text by design. Any allowlist regex strict enough to block attacks would also drop legitimate beliefs; render-time escape (mitigation 2) is the right shape.Both declinations are on lines 103-116 of the existing spec; this PR just makes the close-out structurally legible (status table reflects ship state, residual risk noted in
LIMITATIONS.md, trust-boundary framing inPHILOSOPHY.md).Test plan
src/touched. No test files touched.Out of scope
Anything that would require re-opening the spec's declined-mitigation reasoning. If new evidence shows the declinations were wrong, that's a fresh spec re-ratification cycle, not a follow-up to this PR.
Summary by Sourcery
Document the shipped state and trust boundary of the UserPromptSubmit hook and its mitigations, and record the residual model-layer risk.
Documentation: