Repository navigation
fix(#1795): run scribe content gates before gates that log content - #1796
fullsend-ai-coder[bot] wants to merge 3 commits into
Conversation
The scribe post-script printed the raw topic or title when a gate that runs before the sensitive-content check rejected a proposal (confidence, invalid existing_issue, missing summary), and when the OMITTED line was logged. The sensitive-content and suspicious-Unicode gates themselves also printed the rejected value via gate_reject. A proposal carrying an email address or hidden Unicode could therefore reach the CI log, including on dry runs. Move both content gates to run right after the public_safe check in the existing-issue and new-issue loops, and reject through gate_reject_content, which logs only the item index and category. The gates also cover omit_reason for topics and labels for new issues, since both are printed later. Rebuilt scripts/post-scribe.sh with bundle-sh.sh. Tests: add post-scribe tests for low-confidence, invalid-number, invalid-confidence, omitted and label cases asserting the sensitive value never appears on stdout and the item is counted as rejected. Update the two sensitive-email tests to the content-gate output. Note: pre-commit could not fetch hook repos (TLS failure in the sandbox); hooks were run directly instead (shellcheck 0.10.0.1, whitespace/EOF/merge-conflict checks, lint-release-cases) and passed. scan-secrets passed. make lint (skillsaw via uvx) could not run, as uvx is unavailable; it does not cover shell scripts. post-scribe tests were run with a bc shim because bc is not installed in the sandbox. Closes #1795 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🤖 Review · Commit: Automated review did not complete for this commit. Check the latest review status before merging. If no review is queued or running, comment |
|
🤖 Finished Review · ✅ Success · Started 6:36 PM UTC · Completed 6:46 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.79 |
|
<!-- fullsend:risk-assessment --> DetailsModifies gate execution order across protected script paths without a feature toggle, but overall risk remains moderate given low git churn, tight alignment with issue #1795, and substantial regression test coverage. Previous run<!-- fullsend:risk-assessment --> DetailsModifies gate execution order across protected script paths without a feature toggle, but overall risk is moderate given tight alignment with issue #1795, extensive regression test additions, and low git churn. |
ReviewFindingsMedium
Low
Earlier findings
Previous run<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJyZXF1ZXN0LWNoYW5nZXMiLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJjYXRlZ29yeSI6ImZhaWwtb3BlbiIsImZpbGUiOiJzY3JpcHRzL3Bvc3Qtc2NyaWJlLnNyYy5zaCIsImlkIjoiZl82MmI2MzI0ZTNlZWQzOTdiIiwibGluZSI6NDA0LCJhY3Rpb25hYmxlIjp0cnVlfV19 --> ReviewFindingsMedium
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 6:48 PM UTC · Completed 6:54 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.88 |
contains_sensitive and contains_suspicious_unicode piped `echo` into an early-exiting reader. Under `set -o pipefail`, large multi-line input (e.g. the unbounded labels and omit_reason fields) made the writer die of SIGPIPE, the pipeline fail, and the helper report "clean", so the value reached the log. Feed the detectors via here-strings instead and add regression tests with oversized omit_reason and label values. Regenerate scripts/post-scribe.sh from the source. Notes: pre-commit could not fetch hook repos (certificate error); ran syntax checks, secret scan and scripts/post-scribe-test.sh directly. The two */low-confidence-rejected tests fail in this sandbox because `bc` is not installed; they fail identically without this change. Addresses #1796 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 1 (bot-triggered)Fixed SIGPIPE fail-open in scribe content gates with regression tests, and resolved the shellcheck SC1003 pre-commit failure in the test script. The two */low-confidence-rejected tests fail in this sandbox only because bc is not installed; they fail identically without these changes. Fixed (2):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 6:56 PM UTC · Completed 7:03 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.57 |
Superseded by updated review
| # Tag characters (U+E0000–E007F), zero-width chars, BOM, bidi overrides/isolates | ||
| echo "${text}" \ | ||
| | perl -e 'binmode(STDIN, q(:encoding(UTF-8))); while (<STDIN>) { if (/[\x{E0000}-\x{E007F}\x{200B}\x{200C}\x{200D}\x{FEFF}\x{202A}-\x{202E}\x{2066}-\x{2069}]/) { exit 0 } } exit 1' \ | ||
| perl -e 'binmode(STDIN, q(:encoding(UTF-8))); while (<STDIN>) { if (/[\x{E0000}-\x{E007F}\x{200B}\x{200C}\x{200D}\x{FEFF}\x{202A}-\x{202E}\x{2066}-\x{2069}]/) { exit 0 } } exit 1' <<<"${text}" \ |
There was a problem hiding this comment.
[low] fail-open
contains_sensitive/contains_suspicious_unicode map grep (status >=2) and Perl (status 127/runtime error) execution failures to "clean" because callers use them as predicates. Pre-existing on base main; this PR only changes input delivery (here-strings). Only a missing binary on the runner is a realistic trigger. Same code in scripts/post-scribe.sh.
Suggested fix: Optional hardening outside this PR's scope: treat status 1 as no-match and reject the item (or fail closed) on any other status; regenerate scripts/post-scribe.sh.
fullsend-ai-review
Bot
added
the
requires-manual-review
Summary
scripts/post-scribe.src.sh(and the bundledscripts/post-scribe.sh) now run the sensitive-content and suspicious-Unicode gates right after thepublic_safecheck. They reject throughgate_reject_content, so a proposal matching those patterns is never printed to the CI log, whichever gate would otherwise have rejected it.Root cause
In both the existing-issue loop and the new-issue loop, several gates ran before the content gates and called
gate_reject "${TOPIC}"/gate_reject "${TITLE}", which logs the raw model-produced value (_gha_sanitizeonly escapes workflow-command characters). For comments these were the invalidexisting_issue, missing-summary and confidence gates. For new issues they were the confidence gates. TheOMITTED:log line did the same. The content gates also printed the value they had rejected because it was sensitive. This happened on dry runs too.Changes
gate_reject_content, which logs only the item index and a category (sensitive content/suspicious Unicode) and counts the item as a content-gate rejection.omit_reason, and the new-issue gates also checklabels. Both are printed later (OMITTED:line, dry-runLabels:line), and the issue asks that labels never be printed when they match.scripts/post-scribe.shwithscripts/bundle-sh.sh.Behavior change: an omitted topic, or one with no
existing_issue, is now counted as a gate rejection when it contains sensitive content or suspicious Unicode. Before, it was logged or skipped.Testing
run_test_content_suppressedhelper inscripts/post-scribe-test.sh. It asserts the expected content-gate line is printed, the forbidden value never appears on stdout, and the item is counted as rejected.confidence: 0.2: sensitive topic, sensitive summary only, suspicious Unicode with an invalidexisting_issue, an omitted sensitive topic, a sensitive new-issue title, a sensitive new-issue body only, suspicious Unicode with invalid confidence, and a sensitive label.mainand pass with this change. Existing tests, including the clean low-confidence rejection, still pass, in both source and bundled modes. The two sensitive-email tests now expect the content-gate output.bundle-sh.shoutput. shellcheck 0.10.0.1 (repo args) andscan-secretspassed.Caveats
pre-commitcould not fetch hook repos in the sandbox (TLS failure). The relevant hooks were run directly instead.bcis not installed in the sandbox, so the post-scribe tests ran with a localbcshim.make lint(skillsaw viauvx) could not run becauseuvxis unavailable. It does not lint shell scripts.🤖 Generated with Claude Code
Closes #1795
Post-script verification
agent/1795-scribe-content-gates-first)f589f2e719ba9bdda0a5915b8d4a86c45e7694e7..HEAD)