chore: add Code of Conduct, control-byte/bidi CI gate, and pre-merge check verifier (#38, #288, #289) - #293
Merged
Conversation
…-018) Seven U+0085 (C1 NEL) bytes were injected into comment text in three test files by the edit tooling decoding backslash-u escapes to live bytes. Six U+2028/U+2029 literals in transform.spec.mjs replace live chars with String.fromCodePoint() so no hazard literal lives in tracked source. - crates/mds-cli/tests/cli_lint.rs (5 occurrences): rewrite comment prose to say U+0085 without embedding the character. Assertions unchanged. - crates/mds-napi/__test__/index.spec.mjs (1): same pattern - crates/mds-wasm/tests/web.rs (1): same pattern - packages/bundler-utils/__test__/transform.spec.mjs (6): replace live U+2028/U+2029 char literals with String.fromCodePoint(0x2028/0x2029) No logic changes; only comment text and string-construction form change. This is the S0 step that must land before the control-byte gate (PR6 S1-S6) so CI is not immediately red on the wave branch. avoids PF-018, applies D-CB4 (fix rather than allowlist) Co-Authored-By: Claude <noreply@anthropic.com>
…ifier (#38, #288, #289) #38 — CODE_OF_CONDUCT.md (Contributor Covenant 2.1): - Added at repo root with deanshrn@gmail.com as enforcement contact - Linked from CONTRIBUTING.md and README.md - Removed satisfied prerequisite from RELEASING.md one-time-prerequisites list - Upstream fixture committed for offline diff verification (D-COC2) #288 — scripts/verify-no-control-bytes.mjs: - Pure Node codepoint scanner; no grep (BSD grep lacks -P, exits 2 silently) - Hazard class: C0 excl. TAB/LF, DEL, C1 at codepoint level (catches U+0085 as 0xC2 0x85), 12 Bidi_Control codepoints (Trojan Source / CVE-2021-42574), U+2028, U+2029, U+FEFF — derived from common/mod.rs with CR/CRLF exception - D-CB5: zero-files-scanned is exit 1, not exit 0 (non-vacuity) - D-CB8: --staged mode reads git index via git cat-file blob, never working tree - D-CB6: two empty allowlists (BINARY_ALLOWLIST, HAZARD_ALLOWLIST); stale entries are self-invalidating (exit 1) - CI: source-hygiene job in ci.yml (pull_request); step in release.yml version-gate (tag pushes); opt-in pre-commit hook at scripts/hooks/pre-commit - 51-test suite covering all ACs including positive controls (ADR-009/PF-013), golden class set (D-CB1a), and staged-vs-working-tree isolation (AC-18) #289 — scripts/verify-pr-checks.mjs: - D-PR1: pure evaluateChecks() function; gh runner injected for offline tests - D-PR2: required contexts read live from branch protection (exit 2 on 404/403); --required-from flag for unprotected wave-branch base - D-PR2a: required contexts resolved against union of check-runs AND statuses - D-PR3: Tier A (required, must be completed+success), Tier B (non-required, failure/cancelled/stale = FAIL), Tier C (legacy statuses, advisory) - D-PR4: zero check-runs = exit 1 (the #239 shape — not a pass) - D-PR4a: filter=latest pinned; pagination bounded at MAX_PAGES=20, exit 2 - D-PR5: PASS emits gh pr merge --squash --match-head-commit <sha> - D-PR6: exit 0 PASS, 1 FAIL, 2 indeterminate; never 0 for "cannot tell" - Fixtures from live API: 113f472 (18 checks, PASS), f168944/#239 (0 checks, FAIL), e9dace1/#240 (0 checks + Snyk error, FAIL) - Partial case: 17 of 18 required contexts present → FAIL naming the absent one applies ADR-009, avoids PF-013, avoids PF-016, avoids PF-017, avoids PF-018 Co-Authored-By: Claude <noreply@anthropic.com>
- verify-no-control-bytes.mjs: drop unused execFileSync/join imports; simplify hexContext double-ternary to template literal; drop redundant entry.staged in read-mode branch (isStaged already covers all staged entries set by getStagedFiles) - verify-pr-checks.mjs: remove dead checkGhVersion function (never called; ghVersion() is the real path); remove spurious async from main() (contains no await) - verify-no-control-bytes.spec.mjs: drop unused mkdirSync import - verify-pr-checks.spec.mjs: drop unused dirname import; remove unused loadStatuses call in 113f472 baseline test (result was loaded but [] was passed to evaluateChecks) All 51 tests pass; scanner passes its own gate.
Scanner + verifier (#288, #289): - Entry-point guard no longer compares import.meta.url to a hand-built file:// string. That comparison is false for any path with a space and for any symlinked path (macOS /tmp, /var/folders), so main() never ran and both tools exited 0 having scanned/verified nothing — a silent pass. Now compared by realpath, with a regression test that runs each script from a spaced path. - evaluateChecks now exits 2 (indeterminate) instead of 0 when the required context set is empty; fetchRequiredContexts exits 2 when a protected branch lists zero required checks, and reads the UNION of contexts[] and checks[]. 'All 0 required contexts passed' was a vacuous green (applies ADR-009). - A required context is now evaluated across EVERY check-run sharing its name; previously a later success masked an earlier failure. - HAZARD_ALLOWLIST entries can now be exercised: scanBuffer reports allowlisted hits so the caller records them, instead of every valid entry reading stale. - --staged mode exits 2 when 'git diff --cached' fails instead of treating the failure as 'nothing staged' and passing the pre-commit hook. Tests: - Replaced the AC-28/AC-29 source-text greps (asserting a file contains the string 'process.exit(2)' proves nothing about reachability) with 16 tests that drive main() through an injected gh runner: 404, 403, stale gh, page cap, truncated page set, empty required set, API error, call budget, filter=latest. - Added AC-17 allowlist cases, AC-20 symlink skip, AC-16 git-not-on-PATH, and entry-point regressions. Mutation-checked: reverting each fix fails tests. - CoC fixture provenance is pinned to a measured sha256 + exact byte count; 'hash.length === 64' is true of every sha256 and asserted nothing. Fixture normalized to be byte-identical to the upstream 2.1 body. Docs: corrected the U+0085 remediation comments, which described the Rust escape form while the assertions check the 6-character JSON escape.
…C-30, scope) AC-6: remove `&& !isStaged` guard from non-vacuity check in verify-no-control-bytes.mjs (D-CB5). A --staged scan that finds zero staged files previously exited 0, printing "Scanned 0 file(s)" — indistinguishable from a working scan of a clean index. Now exits 1 unconditionally when fileEntries is empty. Test added. AC-16: change exit 2 → exit 1 for 'git not on PATH' and 'not inside a git work tree' in verify-no-control-bytes.mjs. AC-16 and D-CB5 mandate exit 1 for all four named failure conditions; these two are known, named failures (fail-closed), not indeterminate tool errors (exit 2). Tests updated; header comment revised. AC-22: evaluateChecks in verify-pr-checks.mjs now names every required context as absent when nChecks===0, instead of returning after the single "zero check-runs" line. The test titled "naming all 6 required contexts" now asserts that all six context strings appear in the failure output. AC-30: pre-compute hexCtx inside the hazard-hit scan loop and store it in hazardHits instead of the raw `buf`. The file buffer is released at the end of each file's iteration rather than being retained until all files are scanned. Test added to prove hex context is reported correctly across multiple hazardous files. Scope: remove unplanned --head-sha override from verify-pr-checks.mjs (USAGE string, header comment, argv parsing, and headShaOverride usage in main()). The plan did not include this flag; it had zero test coverage and no provenance note. Self-verification: positive control (planted 0x1B) still exits 1; full-tree scan of 514 files / 4,609,419 bytes exits 0. Co-Authored-By: Claude <noreply@anthropic.com>
Explicit enumeration at ci.yml:331 deviated from plan S2/AC-19 and the established repo convention (7 of 8 node --test call sites use the *.spec.mjs glob). Any spec file added to scripts/__test__/ in the future would be silently skipped — the same "gate that matches nothing" failure mode this PR exists to eliminate, one layer up. Bash on ubuntu-latest expands the glob before passing arguments to node; a non-matching glob (no .spec.mjs files) makes node --test error rather than silently succeed, so the gate remains fail-closed. Co-Authored-By: Claude <noreply@anthropic.com>
…ent exit codes D-CB5 requires the gate to fail closed. Previously the hook printed a 'skipping' message and exited 0 when scripts/verify-no-control-bytes.mjs was not found — meaning a commit that deletes or renames the scanner silently disabled the local gate. Change to exit 1 with an actionable message directing the developer to restore the script or pass --no-verify deliberately. Also correct the exit-code header: (a) exit 1 now covers both the hazard-byte and missing-scanner cases, and (b) document the exit-2 path that propagates from the scanner's three-value contract but was previously undocumented in the hook header. Co-Authored-By: Claude <noreply@anthropic.com>
…tore BREAKING-first order The PR6 commit introduced a second `### Added` heading at the top of the [Unreleased] block, ahead of all the `### **BREAKING**` sections. This violated the file's established ordering (both [Unreleased] and [0.3.0] lead with BREAKING) and left a duplicate heading (one already existed at line 452). Fix: remove the stray `### Added` block from the top of [Unreleased]; merge its three bullets (Code of Conduct #38, source-hygiene gate #288, pre-merge verifier #289) into the existing `### Added` section at the end of the block, just before `### Changed`. Co-Authored-By: Claude <noreply@anthropic.com>
The version-gate job in release.yml ran the source-hygiene scanner (verify-no-control-bytes.mjs) but not the positive-control and class- completeness suite (scripts/__test__/*.spec.mjs). Because ci.yml does not trigger on tag pushes, the suite that pins HAZARD_RANGES (D-CB1a) never executed on the release path. A regression that silently narrowed the hazard class would exit 0 in the release gate while being caught on PRs and pushes to main. The fix is one additional step — the same glob ci.yml uses — added immediately after the scanner step. Applies ADR-009 / avoids PF-013: a completeness claim must be backed by a positive control that detects the hostile artifact when present. Co-Authored-By: Claude <noreply@anthropic.com>
CONTRIBUTING.md hazard-class description (lines 82-86) misplaced U+2028, U+2029, and U+FEFF inside the twelve Bidi_Control=Yes codepoints. They are not Bidi_Control members; U+061C is. Corrected to: 'the twelve Unicode Bidi_Control=Yes codepoints including U+061C (Trojan Source, CVE-2021-42574), plus U+2028 (LS), U+2029 (PS), and U+FEFF (BOM).' This matches verify-no-control-bytes.mjs and CHANGELOG.md, which both already stated it correctly. CONTRIBUTING.md Merging section exit-code paragraph and CHANGELOG.md entry for #289 both documented exit 1 as only 'a required context is missing or not successful'. This understated the contract: exit 1 also fires for Tier B (any non-required check-run concluded failure/cancelled/timed_out/ action_required/stale) and for zero check-runs. Added Tier A/B/C taxonomy. Added a 'Tier B is load-bearing' note because source-hygiene is NOT a required branch-protection context, making Tier B the only binding mechanism for admin merges. CONTRIBUTING.md Scope paragraph now states two additional limitations: (a) source-hygiene is not a required context so --admin bypasses it outright and Tier B is the binding mechanism, (b) Tier B skips queued/in_progress check-runs, so a verifier pass issued while source-hygiene is still running has verified nothing about source hygiene. CONTRIBUTING.md Source hygiene section now documents the scanner's 0/1/2 exit-code contract, matching the verifier's documented contract in the same file. Co-Authored-By: Claude <noreply@anthropic.com>
… pre-flight Root package.json already has the script (landed in HEAD), CONTRIBUTING.md already references it, but RELEASING.md pre-flight was missing it. Any contributor who edits the gate scripts locally and breaks detection gets zero signal until CI. Now both normative docs list the local invocation path. Also expand AC-15 spec to cover the PF-018 backslash-u-escape vector across all gate scripts and spec files, not just the scanner source. Builds the detection pattern from char codes so the test cannot trip its own new rule. Co-Authored-By: Claude <noreply@anthropic.com>
The spec file includes itself in the AC-15 fileSet (along with the other
gate scripts). The HEAD version used src.includes('grep -P') to check
for the forbidden grep flag, but that string literal is present in the
spec file itself — causing the test to fail its own self-check.
Fix: build the forbidden-flag string by concatenation ('grep' + ' -P')
and build the test description without the contiguous substring, so the
spec file passes its own check.
Avoids PF-018 (edit tooling injects live bytes/strings that trip the
very gates they guard).
Co-Authored-By: Claude <noreply@anthropic.com>
…ation Move the AC-1/AC-2 Code of Conduct describe block from scripts/__test__/verify-pr-checks.spec.mjs into its own scripts/__test__/code-of-conduct.spec.mjs so the repo's one-spec-per-module convention is upheld and the CI glob (scripts/__test__/*.spec.mjs, already in place) picks it up automatically. Also strengthens the provenance comment: adds the exact upstream URL, the awk front-matter-stripping command, and the unstripped upstream digest at capture time so a reviewer can re-derive FIXTURE_SHA256 independently rather than trusting the self-measured constant (applies ADR-009, avoids PF-013). Verified 2026-08-13: the strip command produces sha256 369bf730.../5478 bytes, matching the committed fixture exactly. Remove now-unused `statSync` and `createHash` imports from verify-pr-checks.spec.mjs. Co-Authored-By: Claude <noreply@anthropic.com>
Finding #1/#5 — deletion-only commits rejected by non-vacuity guard: In --staged mode, getStagedFiles() uses --diff-filter=ACMR which excludes deletions. A deletion-only commit yields zero ACMR-filtered paths, tripping the D-CB5 non-vacuity guard (exit 1) and blocking a valid `git rm` commit. Finding #2 (HIGH) — git commit --amend --no-edit rejected: During amend the index equals HEAD, so `git diff --cached --diff-filter=ACMR` is legitimately empty. The guard exited 1, blocking a routine workflow and training contributors to reach for --no-verify. Fix: D-CB5 non-vacuity guard now applies to full-tree mode only. In --staged mode, when the ACMR-filtered set is empty the scanner runs an unfiltered `git diff --cached --name-only -z` check and exits 0 with an explicit message: '0 content-bearing staged paths (N deletion(s)) — nothing to scan' for the deletion-only case, or 'no staged content — nothing to scan' for amend/empty. Exit 1 is retained only for the full-tree scan (broken path discovery) and for genuine tool failures (exit 2). Finding #3 — decodeUtf8 materialised entire file as heap-object array: Removed decodeUtf8(); fused UTF-8 decode + hazard check into a single inline pass in scanBuffer(). Only hit records are allocated. Eliminates the ~53x heap amplification measured on package-lock.json (148 KB file → 7.5 MB heapUsed delta). The nextCp CRLF look-ahead uses buf[nextStart] (lead byte), which is correct because isHazardous() only compares nextCp to 0x0A, and LF is ASCII. Finding #4 — one git cat-file subprocess per staged file: Replaced readIndexBlob() (N per-file spawns) with readAllIndexBlobs() which uses `git cat-file --batch` to read all staged blobs in a single subprocess. Measured: 300 staged files at ~11 ms/spawn → ~3.4 s eliminated. Tests: updated AC-6 --staged test to expect exit 0; added deletion-only commit test; incorporated prior session's AC-15 grep-P / backslash-u test. All 37 tests pass. Full-tree scan: 515 files, 4,629,254 bytes, 0 hits. Co-Authored-By: Claude <noreply@anthropic.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
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.
Summary
CODE_OF_CONDUCT.md(Contributor Covenant 2.1) withdeanshrn@gmail.comas enforcement contact, linked from CONTRIBUTING.md and README.md, prerequisite removed from RELEASING.md.scripts/verify-no-control-bytes.mjsscans every tracked file for hazardous codepoints (C0, DEL, C1 at codepoint level, 12 Bidi_Control chars including U+061C, U+2028/U+2029, U+FEFF). Runs on every PR and tag push. Also remediates 13 live hazard bytes already in tracked source (PF-018 had injected U+0085 into 3 test files; U+2028/U+2029 were literal chars in transform.spec.mjs).scripts/verify-pr-checks.mjsguards against PF-017 (cancelled CI run reads as "not failing" togh pr merge --admin). Reads required contexts live from branch protection, requires each to becompleted+success, emits a SHA-pinned merge command on pass.This PR must land first in the wave: it remediates 13 live hazard codepoints and adds the source-hygiene gate that three later PRs (which author control-character tests) depend on.
Changes
New files:
CODE_OF_CONDUCT.md— Contributor Covenant 2.1scripts/verify-no-control-bytes.mjs— hazard scanner (D-CB1..D-CB8)scripts/verify-pr-checks.mjs— pre-merge verifier (D-PR1..D-PR6)scripts/hooks/pre-commit— opt-in hook (mode 100755); reads git indexscripts/__test__/verify-no-control-bytes.spec.mjs— 51 tests, all ACsscripts/__test__/verify-pr-checks.spec.mjs— evaluateChecks offline testsscripts/__test__/fixtures/— 7 fixture files (upstream CoC, live API snapshots)Modified files:
crates/mds-cli/tests/cli_lint.rs— 5 U+0085 comments rewrittencrates/mds-napi/__test__/index.spec.mjs— 1 U+0085 comment rewrittencrates/mds-wasm/tests/web.rs— 1 U+0085 comment rewrittenpackages/bundler-utils/__test__/transform.spec.mjs— 6 live U+2028/U+2029 replaced with String.fromCodePoint().github/workflows/ci.yml—source-hygienejob added.github/workflows/release.yml— scanner step inversion-gatejob.github/PULL_REQUEST_TEMPLATE.md— hygiene and pre-merge verifier checklist itemsCONTRIBUTING.md— Source hygiene section + Merging section + CoC linkREADME.md— CoC link added to Contributing sectionRELEASING.md— satisfied prerequisite removed; pre-flight commands addedCLAUDE.md— two gotcha bullets for the new gatesCHANGELOG.md— [Unreleased] entries for Add CODE_OF_CONDUCT.md (Contributor Covenant 2.1) #38, ci: reject control bytes and bidi codepoints in tracked source (authoring toolchain injects live ESC) #288, process: CI checks can be cancelled and an --admin squash-merge will not notice #289Breaking Changes
None. No published wire contracts change. The only
crates/edits are comment text in test files (not compiled into any artifact). WASM size guard is unaffected.Reviewer Focus Areas
gh pr checks --requiredexits 0 on it, this tool exits 1).Commits
87423ddfix: remediate 13 live hazard codepoints injected by edit tooling (PF-018)2ed7de6feat: add Code of Conduct, control-byte/bidi CI gate, and pre-merge check verifier (Add CODE_OF_CONDUCT.md (Contributor Covenant 2.1) #38, ci: reject control bytes and bidi codepoints in tracked source (authoring toolchain injects live ESC) #288, process: CI checks can be cancelled and an --admin squash-merge will not notice #289)Closes #38, #288, #289