repair(governance): enforce scoped doctoring Security Notes - #1204
seonghobae wants to merge 26 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='5066b03ae60ca979d043b92178402392e3edabdf'
base='314ddeae7b775a4957594b599358c8255617eb2e'
echo '=== commit identity ==='
git rev-parse HEAD
git cat-file -t "$head"
git cat-file -t "$base"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head"
echo '=== exact name-status delta ==='
git diff --name-status "$base" "$head"
echo '=== exact patch ==='
git diff --no-ext-diff --unified=80 "$base" "$head" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/test_verify_security_notes.py \
scripts/harness/quickcheck.sh
echo '=== checker structure at exact head ==='
ast-grep outline scripts/checks/verify_security_notes.py --items all || true
echo '=== checker content at exact head ==='
cat -n scripts/checks/verify_security_notes.py
echo '=== test structure at exact head ==='
ast-grep outline scripts/checks/test_verify_security_notes.py --items all || true
echo '=== test content at exact head ==='
cat -n scripts/checks/test_verify_security_notes.py
echo '=== quickcheck relevant calls ==='
rg -n -C 8 'verify_security_notes|test_verify_security_notes|Security Notes' scripts/harness/quickcheck.sh
echo '=== baseline checker content ==='
git show "$base:scripts/checks/verify_security_notes.py" | cat -nLength of output: 22554 🏁 Script executed: #!/bin/bash
set -euo pipefail
head='5066b03ae60ca979d043b92178402392e3edabdf'
base='314ddeae7b775a4957594b599358c8255617eb2e'
echo '=== registry target in exact head tree ==='
git ls-tree -r --name-only "$head" -- docs/doctoring | sed -n '/sidebar-disabled-tooltips\.md$/p'
echo '=== doctoring markdown files and Security Notes/trust-boundary headings ==='
if [ -d docs/doctoring ]; then
fd -e md . docs/doctoring -x sh -c '
printf "\n--- %s ---\n" "$1"
rg -n -i -C 2 "^[[:space:]]{0,3}#{1,6}[[:space:]]+.*(security notes|trust.boundary)|security notes|trust.boundary" "$1" || true
' sh {}
fi
echo '=== plan files containing References and Security Notes/subsection headings ==='
if [ -d docs/plans ]; then
fd -e md . docs/plans -x sh -c '
if rg -qi "^[[:space:]]{0,3}##[[:space:]]+references|security notes" "$1"; then
printf "\n--- %s ---\n" "$1"
rg -n -i -C 2 "^[[:space:]]{0,3}#{1,6}[[:space:]]+(security notes|references|attack surface|trust boundary|mitigations|test points|realistic threats|remaining risk)" "$1" || true
fi
' sh {}
fi
echo '=== all direct checker calls and imports ==='
rg -n -C 3 '(find_security_notes_violations|security_notes_section|verify_security_notes\.py)' \
-g '*.py' -g '*.sh' -g '*.md' .
echo '=== baseline-vs-head function body, whitespace visible ==='
git diff --no-ext-diff --word-diff=porcelain "$base" "$head" -- scripts/checks/verify_security_notes.py
echo '=== test cases that exercise heading/body placement ==='
rg -n -C 4 '(Security Notes|trust.boundary|References|Approaches considered|Decision)' \
scripts/checks/test_verify_security_notes.pyLength of output: 31871
The source-controlled
The new quickcheck ordering is correct. It runs the policy regression immediately before the production Security Notes gate. You are interacting with an AI system. |
|
@coderabbitai review Fresh review request for exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='47595c4c7bae985068878ba2a816e9e978da6a73'
base='314ddeae7b775a4957594b599358c8255617eb2e'
echo '=== commit identity and ancestry ==='
git cat-file -t "$head"
git cat-file -t "$base"
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base_is_ancestor=yes'
echo '=== exact delta ==='
git diff --name-status "$base" "$head"
git diff --no-ext-diff --unified=100 "$base" "$head" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/test_verify_security_notes.py \
scripts/harness/quickcheck.sh
echo '=== exact-head checker outline and contents ==='
ast-grep outline scripts/checks/verify_security_notes.py --items all || true
cat -n scripts/checks/verify_security_notes.py
echo '=== protected-base checker contents ==='
git show "$base:scripts/checks/verify_security_notes.py" | cat -n
echo '=== exact-head regression tests ==='
ast-grep outline scripts/checks/test_verify_security_notes.py --items all || true
cat -n scripts/checks/test_verify_security_notes.py
echo '=== quickcheck integration ==='
rg -n -C 8 'test_verify_security_notes|verify_security_notes' scripts/harness/quickcheck.sh
echo '=== registered doctoring target existence in exact head ==='
git ls-tree -r --name-only "$head" -- docs/doctoring | rg -n '(^|/)sidebar-disabled-tooltips\.md$' || true
echo '=== all direct checker references in exact head ==='
rg -n -C 3 '\b(find_security_notes_violations|security_notes_section|DOCTORING_SECURITY_NOTES_REQUIRED)\b' \
scripts/checks scripts/harnessLength of output: 29266
I found one blocking issue.
The registry is a source-controlled, explicit scope marker. The The plan parser at Lines 30-47 and the six required subsection strings are unchanged from the protected base. The existing |
|
@coderabbitai review Fresh exact-head review for |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='7ec499bae3ea4016f57fc58c4816edad4b8d8d8d8f'
base='314ddeae7b775a4957594b599358c8255617eb2e'
# Correct the requested head variable from the authoritative user-provided hash.
head='7ec499bae3ea4016f57fc58c4816edad4b8d8d8f'
echo '=== commit identity and ancestry ==='
git cat-file -t "$head"
git cat-file -t "$base"
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'protected_base_is_ancestor=yes'
echo '=== exact protected-base delta ==='
git diff --name-status "$base" "$head"
git diff --no-ext-diff --unified=120 "$base" "$head" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/test_verify_security_notes.py \
scripts/harness/quickcheck.sh
echo '=== exact-head checker structure and content ==='
ast-grep outline scripts/checks/verify_security_notes.py --items all || true
cat -n scripts/checks/verify_security_notes.py
echo '=== protected-base checker content ==='
git show "$base:scripts/checks/verify_security_notes.py" | cat -n
echo '=== exact-head regression test structure and content ==='
ast-grep outline scripts/checks/test_verify_security_notes.py --items all || true
cat -n scripts/checks/test_verify_security_notes.py
echo '=== exact-head quickcheck integration ==='
rg -n -C 10 '\btest_verify_security_notes\.py\b|\bverify_security_notes\.py\b' \
scripts/harness/quickcheck.sh
echo '=== registered file presence in exact-head tree ==='
git ls-tree -r --name-only "$head" -- docs/doctoring | \
rg '(^|/)sidebar-disabled-tooltips\.md$' || true
echo '=== checker call sites and policy constants ==='
rg -n -C 4 \
'\b(DOCTORING_SECURITY_NOTES_REQUIRED|security_notes_section|doctoring_security_notes_section|find_security_notes_violations)\b' \
scripts/checks scripts/harness
echo '=== markdown fence and heading-related test fixtures ==='
rg -n -i -C 6 \
'(fence|```|Security Notes|trust.boundary|References|Attack surface|Realistic threats|Remaining risk)' \
scripts/checks/test_verify_security_notes.pyLength of output: 37866
I found one blocking issue.
The other requested properties are correct:
|
|
@coderabbitai review Fresh exact-head review for |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='58d76aab5dd614375758af3c143a79e8c78712e8'
base='314ddeae7b775a4957594b599358c8255617eb2e'
echo '=== exact commit identity and ancestry ==='
git cat-file -t "$head"
git cat-file -t "$base"
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'protected_base_is_ancestor=yes'
echo '=== exact protected-base changed-file scope ==='
git diff --name-status "$base" "$head"
echo '--- changed paths only ---'
git diff --name-only "$base" "$head"
echo '=== exact patch for stated governance files ==='
git diff --no-ext-diff --unified=120 "$base" "$head" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/test_verify_security_notes.py \
scripts/harness/quickcheck.sh
echo '=== exact-head checker outline ==='
ast-grep outline scripts/checks/verify_security_notes.py --items all || true
echo '=== exact-head checker ==='
git show "$head:scripts/checks/verify_security_notes.py" | cat -n
echo '=== protected-base checker ==='
git show "$base:scripts/checks/verify_security_notes.py" | cat -n
echo '=== exact-head regression tests ==='
ast-grep outline scripts/checks/test_verify_security_notes.py --items all || true
git show "$head:scripts/checks/test_verify_security_notes.py" | cat -n
echo '=== quickcheck Security Notes ordering ==='
git show "$head:scripts/harness/quickcheck.sh" | \
rg -n -C 10 '\btest_verify_security_notes\.py\b|\bverify_security_notes\.py\b'
echo '=== registry target presence in exact-head tree ==='
git ls-tree -r --name-only "$head" -- docs/doctoring | \
rg '(^|/)sidebar-disabled-tooltips\.md$' || true
echo '=== direct checker call sites at exact head ==='
git grep -n -I -E \
'(DOCTORING_SECURITY_NOTES_REQUIRED|security_notes_section|doctoring_security_notes_section|find_security_notes_violations)' \
"$head" -- scripts/checks scripts/harness || true
echo '=== parser and fixture lines related to Markdown code and section boundaries ==='
git show "$head:scripts/checks/test_verify_security_notes.py" | \
rg -n -i -C 8 \
'(fence|indented|code block|```|Security Notes|Trust boundary|References|Attack surface|Realistic threats|Remaining risk)' || trueLength of output: 33635
I found one blocking issue.
The requested repairs are otherwise present:
|
|
@coderabbitai review Fresh exact-head review for |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='21d8856f95e93baf93511d4162438760f2238981'
base='314ddeae7b775a4957594b599358c8255617eb2e'
echo '=== exact commit identity and ancestry ==='
git cat-file -t "$head"
git cat-file -t "$base"
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'protected_base_is_ancestor=yes'
echo '=== protected-base changed-file scope ==='
git diff --name-status "$base" "$head"
echo '--- changed paths ---'
git diff --name-only "$base" "$head"
echo '=== exact patch: governance files ==='
git diff --no-ext-diff --unified=120 "$base" "$head" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/test_verify_security_notes.py \
scripts/harness/quickcheck.sh
echo '=== exact-head checker outline ==='
git show "$head:scripts/checks/verify_security_notes.py" > /tmp/verify_security_notes.py
ast-grep outline /tmp/verify_security_notes.py --items all || true
echo '=== exact-head checker ==='
cat -n /tmp/verify_security_notes.py
echo '=== protected-base checker ==='
git show "$base:scripts/checks/verify_security_notes.py" | cat -n
echo '=== exact-head regression tests ==='
git show "$head:scripts/checks/test_verify_security_notes.py" | cat -n
echo '=== quickcheck ordering ==='
git show "$head:scripts/harness/quickcheck.sh" | \
rg -n -C 10 '\btest_verify_security_notes\.py\b|\bverify_security_notes\.py\b'
echo '=== registered doctoring document in exact tree ==='
git ls-tree -r --name-only "$head" -- docs/doctoring | \
rg '(^|/)sidebar-disabled-tooltips\.md$' || true
echo '=== exact code-filtering and doctoring-policy locations ==='
git show "$head:scripts/checks/verify_security_notes.py" | \
rg -n -C 8 \
'(DOCTORING_SECURITY_NOTES_REQUIRED|doctoring_markdown_lines|doctoring_security_notes_section|Trust boundary|fenced|indent|path\.exists|security_notes_section)' || true
echo '=== exact RED fixtures for code blocks and trust boundary ==='
git show "$head:scripts/checks/test_verify_security_notes.py" | \
rg -n -i -C 8 \
'(fenced|indented|code block|```|Security Notes|Trust boundary|heading.only|token.only|short.fragment|narrative)' || true
echo '=== plan extractor equality after normalizing header line numbers ==='
git diff --no-ext-diff -U0 "$base" "$head" -- scripts/checks/verify_security_notes.py | \
rg -n -C 3 'security_notes_section|required_subsections|docs/plans' || trueLength of output: 43440
I found no blocking issue.
I did not execute repository code in the inspection sandbox. You are interacting with an AI system. |
|
@coderabbitai review Please review only current exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='b217087f7697dbc255cd31688ad5868fd489ee15'
base='314ddeae7b775a4957594b599358c8255617eb2e'
echo '=== exact commit identity and ancestry ==='
git cat-file -t "$head"
git cat-file -t "$base"
git show -s --format='head=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'protected_base_is_ancestor=yes'
echo '=== exact protected-base scope ==='
git diff --name-status "$base" "$head"
git diff --name-only "$base" "$head"
echo '=== exact governance patch ==='
git diff --no-ext-diff --unified=140 "$base" "$head" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/test_verify_security_notes.py \
scripts/harness/quickcheck.sh
echo '=== exact-head checker structure ==='
git show "$head:scripts/checks/verify_security_notes.py" > /tmp/verify_security_notes.py
ast-grep outline /tmp/verify_security_notes.py --items all || true
echo '=== exact-head checker ==='
cat -n /tmp/verify_security_notes.py
echo '=== protected-base checker ==='
git show "$base:scripts/checks/verify_security_notes.py" | cat -n
echo '=== exact-head test module ==='
git show "$head:scripts/checks/test_verify_security_notes.py" | cat -n
echo '=== quickcheck Security Notes ordering ==='
git show "$head:scripts/harness/quickcheck.sh" | \
rg -n -C 10 '\btest_verify_security_notes\.py\b|\bverify_security_notes\.py\b'
echo '=== registered doctoring document in exact tree ==='
git ls-tree -r --name-only "$head" -- docs/doctoring | \
rg '(^|/)sidebar-disabled-tooltips\.md$' || true
echo '=== exact Markdown filtering and HTML-comment fixtures ==='
git show "$head:scripts/checks/verify_security_notes.py" | \
rg -n -i -C 12 \
'(doctoring_markdown_lines|comment|fenced|indent|Security Notes|Trust boundary|path\.exists|security_notes_section)' || true
git show "$head:scripts/checks/test_verify_security_notes.py" | \
rg -n -i -C 10 \
'(html|comment|multiline|fenced|indented|Security Notes|Trust boundary|References)' || true
echo '=== plan extractor diff check ==='
git diff --no-ext-diff -U0 "$base" "$head" -- scripts/checks/verify_security_notes.py | \
rg -n -C 4 '(security_notes_section|required_subsections|docs/plans)' || true
echo '=== direct policy references in exact tree ==='
git grep -n -I -E \
'(DOCTORING_SECURITY_NOTES_REQUIRED|doctoring_markdown_lines|doctoring_security_notes_section|find_security_notes_violations)' \
"$head" -- scripts/checks scripts/harness || trueLength of output: 52247
I found no actionable issue.
I performed static inspection only. I did not treat queued or non-terminal checks as passing evidence. You are interacting with an AI system. |
seonghobae
added
maintenance
priority: high
ChatGPT Codex Connector
|
@opencode-agent review Please review exact head |
|
@cwl-noema-review Review exact head |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| "remaining risk", | ||
| ] | ||
| FENCE_OPEN_RE = re.compile(r"^ {0,3}(`{3,}|~{3,})(.*)$") | ||
| MARKDOWN_HEADING_RE = re.compile(r"^ {0,3}(#{1,6})(?:[ \t]+|$)(.*?)\s*$") |
1 participant
Governance finding
Closes #1203 only after this repository-policy change reaches protected
develop.Protected base is
develop@314ddeae7b775a4957594b599358c8255617eb2e; current exact head is8dda972f182086e16d0152a59e76d2f081fcf2b0. This remains the canonical writer forscripts/checks/verify_security_notes.py, its focused regression suite, and the additive quickcheck invocation. Product lanes must not carry a second copy of this parser.Selected doctoring documents opt in to a rendered level-two
Security Notessection containing an explicit### Trust boundarycontract.sidebar-disabled-tooltips.mdremains the first registered document; absent preregistered files stay ignored until their product owner reaches the checked tree.Current RED → causal repairs
Fresh CommonMark review found three related admission/boundary defects after the earlier raw-HTML unification.
Type-2 HTML-comment termination line.
_remove_html_comment_content()could expose<!-- -->## Security Notesor<!--\n-->## Security Notesas a policy heading even though CommonMark keeps the whole-->termination line inside the raw HTML block. REDad3adf085c438d1dbab113375c41e34b1383047eadds same-line and multiline block-comment regressions; GREEN5251fc14ffa4898eee39533d9da20bb5d9265835admits block-start comments through the raw-HTML state machine before inline-comment stripping.Multiline inline-comment continuation. A comment beginning after visible paragraph text could continue onto the next line and have the parser remove
-->while promoting a trailing## Security Notessuffix as a block heading. RED55bbc22d501cc2928f243cd7a53504ad42e22ac6captures that continuation case; GREENc03399ab248ec5a9336cacefaae53728e623917dconservatively excludes the full line when it begins while an inline comment is still open, preventing comment continuation text from manufacturing block-level governance evidence.Doctoring section ownership.
doctoring_security_notes_section()stopped only on a later textual##prefix, so a level-one# Appendixdid not close the level-two## Security Notessection. A### Trust boundaryunder that appendix could therefore backfill the earlier Security Notes contract. RED8e42cf81c2a737a02de60b39ffd0f3b9b7da1b72fixes the expected failure; GREEN8dda972f182086e16d0152a59e76d2f081fcf2b0reuses the shared rendered-heading parser, requires an actual level-twoSecurity Notesheading, and terminates its evidence at the next heading of level two or higher.These REDs are source-level regressions; no hosted failing run is claimed for them. The checker is deliberately a conservative policy-admission parser, not a full CommonMark renderer. Ambiguous unsupported constructs fail closed rather than becoming review-visible security evidence.
Reference: CommonMark 0.31.2, §4.6 HTML blocks — https://spec.commonmark.org/0.31.2/#html-blocks
Earlier contracts retained
Earlier canonical RED
8871a5ae22873835271e7e42b8cbf973fd2eb1f7→ GREENa96ba84c8bd73232e228fabd41ef9726e788ac1dunified plan and doctoring raw-HTML admission throughmarkdown_policy_lines(). Plan REDdba7a961e4371e34f61567147ae26ce881b4bf87→ GREEN88cba336d868b184c8740e84b9fdaa3c6274fba9→ edge contract67428375e99c72f069bfafb7b149a35e8b6355bbstill covers fenced/indented code, hidden HTML/comment evidence, fake headings, peer-section backfill, ATX indentation and closing-hash boundaries. Earlier CodeRabbit findings on bare trust-boundary prose and plan extraction also remain repaired.The plan scope remains
docs/plans/*.md; this PR does not expand the six-subsection plan template to all doctoring/traceability documents. Doctoring remains opt-in and keeps its narrower substantive trust-boundary rule.Single-writer consolidation
Project Persistence #970 temporarily accumulated a parallel generic Security Notes parser/test lineage while investigating the same class of issue. That was a single-writer violation. #970 ordinary descendants restored the generic checker to protected
developand removed its generic governance regression files; historical #970 parser commits remain ancestry only. This PR owns exactly:scripts/checks/verify_security_notes.pyscripts/checks/test_verify_security_notes.pyscripts/harness/quickcheck.shNo Tooltip/UI source, product-persistence code, dependency, lockfile, model, audio/MIR code, release workflow, required status, or gate threshold is changed.
Evidence boundary
Every source movement invalidates predecessor hosted evidence. Exact
8dda972f182086e16d0152a59e76d2f081fcf2b0must obtain its own repository/central checks. #1176 remains the one-file formatter prerequisite and must reach protected ancestry before ordinary reconciliation; its delta must not be copied here.A qualifying independent non-author current-head formal approval is also required. Automated review comments, predecessor approvals, queued/pending checks, self/admin approval, mention receipts, or synthetic statuses are not merge authority.
Security Notes
Documentation source is untrusted policy input. The attack surface is parser ambiguity allowing hidden/example text or mis-bounded sections to impersonate review-visible governance evidence. The trust boundary is quickcheck admission for plan and registered doctoring evidence. Mitigations are one shared rendered-Markdown admission boundary, explicit heading-level ownership, exclusion of code/comments/raw HTML including whole termination lines and continuation lines, bounded doctoring registration, and focused regressions. Test points cover hidden headings/subsection phrases, peer and higher-level section leakage, ATX/closing-hash edges, representative raw-HTML classes, block-comment termination tails, multiline inline-comment continuation, and existing doctoring behavior. Realistic threats are generated or hand-written documentation satisfying a security gate without visible reviewer evidence. Remaining risk is that the checker is intentionally conservative rather than a full renderer; ambiguous constructs must fail closed and be repaired in this canonical governance owner.
Keep Draft until #1176 reaches protected ancestry, this branch is ordinarily reconciled, one unchanged final head/base has all applicable repository/central checks terminal-success, valid findings are zero, and qualifying independent non-author review exists. No self-approval, bypass, force-push, destructive rebase, no-op retrigger, synthetic status, or gate weakening.