[PR #224] [MERGED] ci(lint): fail on org repo literals not on a committed allowlist #232

New issue

Closed

opened 2026-10-26 22:37:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:37:37 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/224
Author: @mattmillerai
Created: 8/25/2026
Status: ✅ Merged
Merged: 9/18/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-8192-org-repo-literal-lint

📝 Commits (10+)

44e24ec ci(lint): fail on org repo literals not on a committed allowlist
057f488 fix(lint): close the normalization, glob and scope holes in the literal lint (BE-8192)
7bdce18 fix(lint): reject either-probe globs, anchor the org token, guard the fallback scan (BE-8192)
67209af fix(lint): restructure the fallback scannability probe to satisfy SC2015 (BE-8192)
30c3d02 fix(lint): reject globbed allowlist entries and align the git probe (BE-8192)
6295758 fix(lint): escape the annotation, bound the hit loop, test git's output (BE-8192)
3a37181 fix(lint): normalize before measuring, close the config-injection scope hole (BE-8192)
eb1bcbe fix(lint): close GIT_CONFIG_PARAMETERS, validate record shape, complete the merged-token guard (BE-8192)
79c907e Merge remote-tracking branch 'origin/main' into matt/be-8192-org-repo-literal-lint
a75b6a0 fix(lint): allowlist repo-name literals main introduced during the merge (BE-8192)

📊 Changes

6 files changed (+2223 additions, -0 deletions)

View changed files

➕ .github/lint/README.md (+257 -0)
➕ .github/lint/check-org-repo-literals.sh (+845 -0)
➕ .github/lint/org-repo-allowlist.txt (+225 -0)
➕ .github/workflows/test-org-repo-literals.yml (+880 -0)
📝 AGENTS.md (+12 -0)
📝 CONTRIBUTING.md (+4 -0)

📄 Description

ELI-5

This repo is public, so writing a private repo's name into a file here publishes it. Until now the only thing stopping that was a rule in AGENTS.md that a reviewer had to remember. This PR turns the rule into a CI check: every Comfy-Org/<name> written anywhere in the tracked tree has to be on a short committed list of names we've confirmed are safe to publish. Anything else fails the build with the file, line and the exact text. Adding a name means editing the list, which shows up in the diff — so publishing a name becomes a decision someone reviewed, not an accident.

The list is an allowlist, never a denylist. A denylist would have to contain the private names in order to match them, so the lint would itself be the leak.

What's in here

.github/lint/check-org-repo-literals.sh — the lint. git greps the tracked tree for org-prefixed repo literals, normalizes each name (case, a .git suffix, trailing periods — not a trailing -/_, which are legal at the end of a real slug), and fails with file:line:match for anything not on the allowlist. Exit 0 clean / 1 findings / 2 configuration error. shellcheck -x clean, works on bash 3.2 and 5.
.github/lint/org-repo-allowlist.txt — the list. One name per line, each with a trailing # comment saying why it's safe, grouped into real public repos / documented exceptions / CODEOWNERS team handles / documentation examples / Hugging Face model namespaces / test fixtures. Team handles live in their own @-scoped namespace, so a CODEOWNERS slug never clears a bare reference to a repo of that name.
.github/lint/README.md — the rule, how to add a name, why this isn't public-repo-hygiene, and the known limitations.
.github/workflows/test-org-repo-literals.yml — runs shellcheck + the lint + four smoke-test steps (45 assertions: exit-status, normalization, the token boundary, the team/repo namespace split, over-broad allowlist entries, the "the scan could not run" configuration errors, the git grep production path with a hostile grep.* config, and — new this round — the workflow-command escaping of a hostile tracked path, the hyphenated sibling owner, a bare-repo root, GIT_DIR in the environment, the per-hit cost bound and the findings cap), with no paths: filter. The leak surface is the whole tree (past leaks sat in scripts/, .github/dependabot.yml and a component README), so gating this on workflow-file paths would exempt exactly the files most likely to carry one.
AGENTS.md / CONTRIBUTING.md — the convention bullet now says the rule is CI-enforced and where the allowlist lives; the Commands/Before-you-open-a-PR blocks gained the command; the "every test workflow is path-filtered" sentence was corrected, since this one deliberately isn't.

Stated limitation

The guard only catches org-prefixed literals. A bare repo name — a cloud-style word written with no Comfy-Org/ in front of it — cannot be linted without committing a denylist that is itself the leak. Bare-name discipline remains with review and AGENTS.md. This is stated in the script header, the README and AGENTS.md as well as here.

Second limitation, same place: the name class is ASCII, so a name whose tail is non-ASCII (a homoglyph dash, a long s, CJK text) is read only as far as its ASCII prefix. The scan is pinned to LC_ALL=C so that limit is identical on every machine instead of a function of the runner's locale — without that pin, grep -i folds U+017F/U+212A into [A-Za-z] in a UTF-8 locale and not in the POSIX one, and the same tree would pass locally and fail on CI. The class cannot simply be widened to high bytes: measured against the tracked tree, 10 literals run into a high byte and only 5 are the deliberate homoglyph fixtures — the rest are an ellipsis, a curly apostrophe and CJK text written flush against a real public name in prose, which widening turns into unfixable findings.

Third limitation: both scan paths pass -I, so a tracked blob git classifies as binary is never read. One .gitattributes line can take a whole file type out of scope. Text is the only surface this lint claims, and the script header and README now say so rather than letting a green run overstate it.

Fourth limitation: only a literal written whole is caught. An org literal assembled at run time — printf '%s/%s' '<org>' '<name>', "${ORG}/<name>" — never matches, so it needs no allowlist edit while the run still reports its scope clean. This is not hypothetical: the smoke tests below use exactly that spelling on purpose, which makes it the house style for org literals in this repo's workflow files. It stays fully human-readable in the source, so review is what catches it.

Fifth limitation: a left boundary is not an owner-name boundary. The org segment must start a token, which excludes Not<org>/x — a different owner whose name ends in ours — but - is legal in a GitHub owner name and is not an identifier character, so the hyphenated sibling Not-<org>/x still satisfies the boundary and is read as one of ours. That is a false positive, not a miss: the failure mode is a red CI on a reference that has nothing to do with us, never a leak. It is kept rather than fixed so the boundary class stays byte-identical to public-repo-hygiene's REPO_REF_RE — a name must not pass one checker and fail the other — and widening it is a change to both. Both spellings are pinned by the smoke tests.

Sixth limitation: a team @ is judged by position, not by grammar. An @ glued to the tail of an identifier (user@<org>/<name>, email-shaped) still reads as a scope, so such a literal can draw on the @-scoped team entries. Reaching a private name through that needs the name spelled exactly like an allowlisted team slug. public-repo-hygiene reads the @ from the same position and carries the same residual.

Judgment calls

A new workflow rather than widening test-workflow-pins.yml. Both were offered. A repo-wide guard bolted onto a workflow named for pin linting would run the Python pin suite on every docs change and read as mis-scoped; a separate check also gives a legible name in the checks list. It is named test-org-repo-literals.yml rather than repo-hygiene.yml to avoid colliding with the existing public-repo-hygiene.yml reusable workflow, which is a different thing.
This is not a second copy of public-repo-hygiene. That checker is the rigorous, org-wide implementation of this same default-deny idea and is what every other repo should adopt. This repo cannot adopt it as a caller: it is that checker's own home, so its tests and docs deliberately commit fake-private fixture names, quote internal collaboration-tool hosts, and carry ticket-style ids by convention — all three of its categories, all intentional. I measured it: running that checker over this tree reports 1232 findings, of which 894 are ticket-style ids, 246 are org repo/team references (its own fixtures, doc examples and the names this allowlist now covers) and 92 are internal collaboration-tool links quoted in its own docs and tests. This lint is the one category this repo can enforce on itself today, and the script header, the README and AGENTS.md all say new detection belongs in the org-wide checker rather than here.
The allowlist is seeded from an actual scan of main, not from a guess. Every entry corresponds to a literal that is really in the tree today, and I read the surrounding context for each one before listing it. The seeding is a superset of what was originally sketched, because the org-wide checker and its fixture corpus landed here after that sketch was written.
Case-insensitive matching, both halves. GitHub resolves owner names case-insensitively, so comfy-org/<name> reaches the same repository; matching only the canonical spelling would be a one-keystroke bypass of a default-deny control. Turning this on surfaced 6 additional literals a case-sensitive scan missed. It also removes the need to list a name twice for its casing.
The org segment must start a token, using public-repo-hygiene's REPO_REF_RE left boundary verbatim rather than a boundary invented here — the two checkers then agree on what counts as a reference, including the trade that _ reads as identifier-continuation. Without it grep -o reads this org's name out of the middle of a different owner's (Not<org>/whatever) and reddens CI on a reference that has nothing to do with us. grep -E has no lookbehind, so the boundary character is consumed by the match and dropped in the loop; the team/scope @ is read from that position. The boundary may itself be a colon (uses:, see:<org>/x), which puts a fourth colon on a file:line:match line, so the right-anchored split detects and corrects that case — pinned by a test asserting the reported LOCATION, not just the exit status.
An over-broad allowlist entry is a configuration error (exit 2), not a blanket clear. One character is otherwise all it takes to neuter the control while still looking like an ordinary allowlist edit. The guard tests behaviour rather than spelling — an entry that clears either of two unrelated probe names is rejected — so it covers **, ?*, [a-z]*, [a-y]*, [!z]* and *[a-z] as well as *, without depending on having enumerated the spellings. Either, not both: requiring both honoured any glob that missed one probe, and [a-y]* is a single keystroke from [a-z]*. The two probes differ in first character, last character and separator, so a glob anchored at any of those positions trips one of them. All seven spellings are pinned by a smoke test.
Team handles are a separate namespace from repo names. An @-prefixed allowlist entry clears only an @-prefixed literal, so a slug listed for a CODEOWNERS fixture does not also clear a literal reference to a private repo of that name. A plain entry still clears either spelling, because @<org>/<name> is also how npm and GitHub Packages write a package scope — @<org>/comfy-cli appears 81 times here as exactly that. This is the same asymmetry public-repo-hygiene documents.
Test-fixture families are enumerated, not globbed. A secret-* line would pre-approve an unbounded set of names on a list whose whole value is that each name was reviewed once. All 58 fixture names that actually appear in the tree are individual lines; a new one reddens CI until someone adds it, which is the intended cost.
The scan fails closed. git grep/grep exit 1 for "no matches" and >1 for a real error; swallowing the latter would report a clean tree because the scan never ran, and a green run reads as coverage.
unreviewed-merges is allowlisted as a documented exception, not as a public repo — it is published by decision as the SOC-2 tracker destination. Its line points at the README and docs/callers/detect-unreviewed-merge.md, which already publish the name, and states the decision rather than a visibility: "X is private" is itself the private detail, and an entry's own line is the one place this scan structurally cannot look (the entry clears it), so the allowlist header now says what belongs there.
The per-hit work is bounded by failing closed, not by truncating. A name past GitHub's 100-character repo-name limit after normalization is reported without being compared, because normalizing an unbounded match is quadratic — measured at over two minutes for one tracked line of <org>/comfy-cli plus 400 KB of periods, against the job's timeout-minutes: 10, which turns a lint into an inconclusive run. Truncate-then-compare would have been the obvious fix and is a widening: that same line would have normalized down onto the allowlisted comfy-cli. An over-long name cannot be a real repo, so reporting it can never clear one. Round 5 moved the order: the documented .git/trailing-period strips now run before the exact test, bounded by 8 bytes of headroom, because measuring first forced <org>/<98-char-name>.git into the over-long branch — which skips both allowlist comparisons, so the finding named a remedy that could not silence it. Past the headroom the peel is still refused rather than run, so the quadratic bound is unchanged. Two assertions cover it — a stopwatch and a verdict check — because the cheap way to pass the stopwatch alone would be to truncate and clear.
The finding line carries a constant unapproved: prefix. A tracked path is untrusted input to the runner, and at column zero a file named ::stop-commands::x.md is a workflow command that suppresses every ::error after it. The prefix makes that unreachable; the annotation's file= value is percent-escaped for the same reason.

Verification of the deny path

This change's user-facing outcome is a build failure, so the premise worth falsifying is "these literals are not legitimately in use". I ran the lint against every tracked file on main rather than only the files this diff touches: it is green, so no existing legitimate reference is denied by it, and nothing had to be scrubbed out of the tree to make it pass. The five smoke tests then prove the opposite direction — that it does fail — on an unlisted name, on a lowercased org spelling, and on a wildcard allowlist entry.

Residual

Bare repo names are still unguarded. A private repo name written without the org prefix passes this lint untouched, and by design always will. That half of the AGENTS.md convention remains review-enforced. If it needs machine enforcement, that is a different mechanism (a secret-backed roster comparison, not a committed denylist) and its own piece of work.
Non-ASCII name tails are read as their ASCII prefix. A name spelled with a homoglyph can clear on an allowlisted prefix here. Three such fixtures already live in the tree and are allowlisted by their prefix with the codepoint named in the comment. Closing this properly needs the offset-aware scan .github/public-repo-hygiene/check_public_repo_hygiene.py already implements; the right fix is to reuse that, not to grow this script.
This repo is still not enrolled in public-repo-hygiene.yml, and this PR does not change that. It has the reusable workflow, the checker, the tests and a caller-bump fleet, but no self-enrollment caller — the same two-step enrollment gap AGENTS.md warns about. Enrolling it would require deciding what to do about the 894 ticket-id findings and the fixture corpus measured above, which is a scoped decision rather than a lint edit. Category 3 is now covered here; categories 1 and 2 are not covered on this repo at all.
AGENTS.md is over its own integrity checker's hard ceiling and this PR makes it slightly longer. It was 305 lines before (ceiling: 200) and is 317 after — python3 .github/agents-md-integrity/check_agents_md.py --root . fails on it, and has been failing on it, because this repo runs that checker for consumers but not on itself. Pre-existing and out of scope here; trimming it is a real piece of work, not a drive-by.
A git failure still falls through to grep -r. The scan branch is now chosen from rev-parse --is-inside-work-tree's output rather than its exit status, which fixes the bare-repo and .git-path roots, but an actual git error (safe.directory refusal, a broken config, no git binary) also leaves the output non-true and takes the fallback, which reads untracked and ignored files. The fallback does at least read $root — which is what the OK line claims — and the branch is unreachable in CI, where the checkout is always a work tree owned by the runner user. Distinguishing "git refused" from "not a repo" is a separate decision about whether a safe.directory refusal should be exit 2.
A runner-level core.attributesFile is still honoured. The four GIT_CONFIG_* injection channels are unset, but that restores git's default config search rather than disabling it, so a core.attributesFile in the runner's $HOME/.gitconfig or /etc/gitconfig still marks files binary and takes them out of git grep -I. Pointing GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM at /dev/null plus GIT_CONFIG_NOSYSTEM=1 would close it, and is deliberately not done: it would also drop a runner's legitimate safe.directory, which makes rev-parse fail and silently takes the grep -r fallback on every run. A PR cannot write the runner's config, so this sits on the trusted side of the same boundary the checkout does.
A newline in a tracked path is refused, not scanned, on the grep -r path. The record-shape and digit checks make it exit 2 rather than report a fabrication, so such a file is never scanned there. On the git path it is scanned normally (git C-quotes the path into one record) and reported with a C-quoted location. Renaming the file is the fix; supporting it would mean NUL-delimited output on both paths.
$hits still buffers the whole scan output before the hit loop starts, and the per-hit loop stays linear in the hit count (limitation 11, now carrying the measurement: ~5.4 ms per hit past the print cap, so the job's timeout-minutes: 10 is reached at roughly 110,000 hits in one scan). That is inherent to capturing the scan's exit status, which is what makes it fail closed; streaming would restructure that. With the per-hit work now linear and the emission capped at 200 printed findings, what remains is memory proportional to one grep -o output.
Two local suites could not be run for want of tools on this machine, and neither is touched by this diff: scripts/area-label/tests/test_lib.sh (needs yq) and .github/coderabbit-config/tests (needs the pinned jsonschema, which is installed from requirements.txt in CI). Both run normally in CI.
Two short ASCII prefixes are still allowlisted as prefixes. comfy-type and mod are the ASCII heads of public-repo-hygiene's deliberate homoglyph fixtures, so a private name spelled <org>/mod<U+2010>… would clear on mod. core-engine, the third, moved into the @-scoped team namespace and no longer clears a bare repo reference at all. Reaching the remaining two needs someone to write a homoglyph on purpose, which the script's TAMPER BOUNDARY block already places outside what this lint controls; real closure is the offset-aware scan public-repo-hygiene implements.

Provenance

Authored by: agent-work loop
Verified: Rounds 1–3: bash .github/lint/check-org-repo-literals.sh on the full tracked tree: 0 findings, with the allowlist file itself in scope; shellcheck -x on the script: clean; actionlint on the workflow: clean. The script re-run under bash 3.2 and bash 5, and under LANG=C, C.UTF-8 and en_US.UTF-8: identical result in all six combinations. All four smoke-test steps replayed locally from the workflow YAML, under both bash 3.2 and bash 5: 26/26 assertions passed at that point — an unlisted name, a lowercased org spelling, a trailing--/_ variant of an allowlisted name, a bare reference spelled like a team slug and a literal glued to a colon all exit 1; a plain allowlisted name, a .git+period spelling, an @-prefixed team slug, a reference naming nothing and an UNHYPHENATED reference to a different owner whose name ends in ours all exit 0 (the hyphenated spelling is limitation 5 above, pinned separately as a known false positive); the allowlist entries *, **, ?*, [a-z]*, [a-y]*, [!z]* and *[a-z] each exit 2; a work tree with no tracked files, an unenterable root, a root holding only binary blobs and an unreadable allowlist each exit 2; on the git grep PRODUCTION path a tracked unlisted name exits 1 and a tracked allowlisted name exits 0; and both the colon-boundary case and a repo configured with hostile grep.column/grep.fullName still report the fixture's own file:line (fixture.md:2). The grep.* pin was confirmed load-bearing by running the same git grep with and without it (f.md:2:4: … vs f.md:2: …). Multibyte boundary characters (an em dash, CJK text) still produce a correctly-located finding. python3 .github/workflow-pins/check_workflow_pins.py: OK, 11 workflows, 0 findings. unittest suites cursor-review 194, agents-md-integrity 46, groom 365, public-repo-hygiene 165, refresh-reviewers 48, workflow-pins 265: all OK. Shell suites bump-callers 437, preflight 278, paths-contract 70, pr-derisk 63: 0 failed (paths-contract needs a __pycache__-free tree — a stale one from running the Python suites fails it, which is the artifact and not the code). The homoglyph measurement quoted above was taken by re-running the scan over the tracked tree with the name class widened to high bytes and diffing the matches. Round 4 re-verified everything from scratch after 6295758: shellcheck -x clean; actionlint on the changed workflow clean; the lint green on the full tracked tree (0 findings) under LANG=C, C.UTF-8 and en_US.UTF-8; python3 .github/workflow-pins/check_workflow_pins.py OK (11 workflows, 0 findings) and its 265 unittests OK. All four smoke-test steps replayed from the workflow YAML under both bash 3.2 and bash 5: 45/45 assertions pass in each, 90 total, zero ::error lines. Step 3 was additionally replayed with id stubbed to report uid 0: 4 of its 6 assertions still run and only the two chmod ones skip, against 1 of 6 before. Each fix was reproduced failing first: ::stop-commands::a,b.md emitted a column-zero workflow command and an unescaped file=; Not-<org>/<unlisted> exited 1 while Not<org>/<unlisted> exited 0; rev-parse --is-inside-work-tree printed false and exited 0 for a bare repo and for a path under .git, both dying with the wrong message; GIT_DIR=b/.git GIT_WORK_TREE=b git -C a grep -l -e '' listed b's files, and end to end a clean root reported a dirty repo's finding; 400 KB of trailing periods ran past two minutes (profiled to 62 s in ${hit##*:} alone, and the two-expansion rewrite ${value%%"${value##*[!.]}"} measured quadratic as well) and now completes in ~2 s. Cap boundary measured exactly: a 100-character name reports "not on the allowlist", a 101-character one "longer than GitHub's 100-character repo-name limit"; a 250-finding fixture prints 200 lines, reports 250 and exits 1. Edge cases re-checked green: a multibyte (em dash, CJK) boundary character, a literal % in a tracked path, a colon in a real directory component, and a colon boundary and a colon in the path together — all still report the correct file:line. Round 5 re-verified from scratch after 3a37181: shellcheck -x clean; actionlint on the changed workflow clean; the lint green on the full tracked tree (0 findings) under bash 3.2 and bash 5; unittest suites cursor-review 194, agents-md-integrity 46, groom 365 all OK and the bump-callers shell suite 437 passed / 0 failed. All five smoke-test steps replayed from the workflow YAML: 63/63 assertions pass (45 before), zero ::error lines. Every new assertion was then replayed against the pre-fix script and fails there, while the four fail-closed controls pass on both, as they must. Each fix was reproduced failing first: <org>/<98-char>.git and <org>/<100-char>. both exited 1 as over-long with the allowlist skipped; GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.attributesFile pointing at one *.md binary line made the unfixed script print the clean-tree OK line and exit 0 for a literal it never read; a tracked file named a:bc<LF>d.md produced ::error file=a,line=./a::… c is not on … — a fabricated literal at a path that does not exist, with the raw path in line=; git -C docs -c grep.fullName=false printed x.md where true prints docs/x.md; and <org>/foo.<org>/private-name scanned as one match named foo.<org>. Performance measured directly: 2000 unapproved literals in one tracked file went 18.4 s → 11.7 s (8.6 ms → 5.4 ms per hit past the print cap, against 2.1 s at 200 hits). Round 6 re-verified after eb1bcbe: shellcheck -x clean (including the one new # shellcheck disable=SC2254), actionlint clean, the lint green on the full tracked tree under bash 3.2 and bash 5, and the other suites unchanged (cursor-review 194, agents-md-integrity 46, groom 365, bump-callers 437 passed / 0 failed). All five smoke steps replayed: 70/70 assertions pass (63 before), and every new one was replayed against the round-5 script and fails there. Each round-6 fix was reproduced failing first: GIT_CONFIG_PARAMETERS="'core.attributesFile=…'" made the round-5 script print the clean-tree OK line and exit 0; a tracked file named a:1:foo<LF>x.md printed the fabricated finding oo at file=a,line=1 for a path that does not exist; the allowlist entries public-name_<org>, public-name<org> and a bare <org> were all accepted; and --root <repo>/docs validated that repo against this repo's allowlist. The git grep C-quoting premise was checked directly ("a:1:foo\nx.md":1:<org>/private-name — one record, not two), and SC2254 was checked against the build CI uses: shellcheck --include=SC2254 exits 0 on the two $org_glob/* patterns and fires on the new $org_glob/) one.
Deviations: none against the requested scope. The allowlist is a superset of the originally sketched seed set (seeded from a real scan instead), a name is not listed twice for its casing (matching is case-insensitive), and the CI wiring is a new minimal workflow rather than a widened test-workflow-pins.yml — both were offered as implementer's choice. Review round 1 changed three defaults from the first pass: fixture families are enumerated rather than globbed, team handles got their own @-scoped namespace, and the over-broad-entry guard tests behaviour rather than the single * spelling. Review round 2 changed four more: the over-broad guard rejects on either probe rather than both, the org segment must start a token (public-repo-hygiene's boundary verbatim, with the colon-boundary parse fix that needs), the grep -r fallback fails closed on a root with nothing scannable, and an unreadable allowlist is exit 2 rather than exit 1 — plus git grep's output-format config is now pinned and a fourth smoke step exercises the git production path. Review round 4 fixed seven of eight threads and answered the eighth on the thread: the claim that [ … ] && longest=… as a loop body's last command aborts under set -e is the opposite of bash's documented behaviour (a command before the final && of an &&/|| list is exempt from -e), measured on bash 3.2 and bash 5 both as bash -e script and in-script, with a bare false body as the contrast that isolates it. Two items from those threads were deliberately not fixed and are recorded under Residual instead of as follow-ups, because neither is reachable in CI: the git-failure fallback and the $hits buffering. Review round 5 fixed all nine threads: the length test moved after normalization (with MAX_STRIP bounding the peel), GIT_CONFIG_COUNT/GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM joined the GIT_DIR unset, a non-numeric line field became a refused parse rather than an escaped finding, grep.fullName was pinned true instead of false, the allowlist loader gained two entry rejections (/, and a name ending in .<org>/-<org>), the per-hit head-slice fold became a glob built once from $ORG, and two stale comments were corrected. One was fixed only in part and is recorded rather than filed: the per-hit loop stays linear in the hit count, and neither suggested bound was taken because both trade the true finding count — the number a red run's summary turns on — for a truncated one; the cost is now stated as limitation 11 with its measurement, and no follow-up ticket was filed because it is not reachable under current code. Review round 6 fixed six of seven threads: GIT_CONFIG_PARAMETERS joined the unset (a fourth injection channel delivering the same fail-open), the record's shape is validated rather than just its line field, the merged-token entry guard became the complete org-suffix test rather than a sample of separators, --root below a work tree now resolves the scanned repo's own allowlist, two tests that were passing for the wrong reason were rewritten to pin what they claim, and the allowlist file's own header — the copy a contributor actually opens — was brought in line with the other two. The seventh was rejected with evidence: SC2254 does not fire on the two $org_glob/* case patterns (CI's required shellcheck -x step passed on that exact file, and --include=SC2254 exits 0), because a pattern ending in * reads as intentional globbing. The mechanism is real, and the new $org_glob/) pattern it does fire on carries a directive. No Linear follow-up was filed this round. Everything not fixed is under Residual above.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/224 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/25/2026 **Status:** ✅ Merged **Merged:** 9/18/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-8192-org-repo-literal-lint` --- ### 📝 Commits (10+) - [`44e24ec`](https://github.com/Comfy-Org/github-workflows/commit/44e24ec31540e5449341f42cb29f7e60cf150ce3) ci(lint): fail on org repo literals not on a committed allowlist - [`057f488`](https://github.com/Comfy-Org/github-workflows/commit/057f4882fa438bf69013c281b227713717d2ea16) fix(lint): close the normalization, glob and scope holes in the literal lint (BE-8192) - [`7bdce18`](https://github.com/Comfy-Org/github-workflows/commit/7bdce189079534f8de9ab7b52149f93ae3d65540) fix(lint): reject either-probe globs, anchor the org token, guard the fallback scan (BE-8192) - [`67209af`](https://github.com/Comfy-Org/github-workflows/commit/67209af229a2217615bf79dbade306e9b60a134e) fix(lint): restructure the fallback scannability probe to satisfy SC2015 (BE-8192) - [`30c3d02`](https://github.com/Comfy-Org/github-workflows/commit/30c3d0285f21882a6dfc7500b15495c61305f3e5) fix(lint): reject globbed allowlist entries and align the git probe (BE-8192) - [`6295758`](https://github.com/Comfy-Org/github-workflows/commit/62957585ef8c1a400fa62d7d2711a7335d096311) fix(lint): escape the annotation, bound the hit loop, test git's output (BE-8192) - [`3a37181`](https://github.com/Comfy-Org/github-workflows/commit/3a3718169562f9c49b51b1682416142e64e50cc0) fix(lint): normalize before measuring, close the config-injection scope hole (BE-8192) - [`eb1bcbe`](https://github.com/Comfy-Org/github-workflows/commit/eb1bcbe9232216a7373dfe290cd84e032a22286c) fix(lint): close GIT_CONFIG_PARAMETERS, validate record shape, complete the merged-token guard (BE-8192) - [`79c907e`](https://github.com/Comfy-Org/github-workflows/commit/79c907eb5fc20ab1269bc7ff4eb98105a9c71a5c) Merge remote-tracking branch 'origin/main' into matt/be-8192-org-repo-literal-lint - [`a75b6a0`](https://github.com/Comfy-Org/github-workflows/commit/a75b6a08444b9a4ef7453220a0a132bca5f54586) fix(lint): allowlist repo-name literals main introduced during the merge (BE-8192) ### 📊 Changes **6 files changed** (+2223 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `.github/lint/README.md` (+257 -0) ➕ `.github/lint/check-org-repo-literals.sh` (+845 -0) ➕ `.github/lint/org-repo-allowlist.txt` (+225 -0) ➕ `.github/workflows/test-org-repo-literals.yml` (+880 -0) 📝 `AGENTS.md` (+12 -0) 📝 `CONTRIBUTING.md` (+4 -0) </details> ### 📄 Description ## ELI-5 This repo is public, so writing a private repo's name into a file here publishes it. Until now the only thing stopping that was a rule in AGENTS.md that a reviewer had to remember. This PR turns the rule into a CI check: every `Comfy-Org/<name>` written anywhere in the tracked tree has to be on a short committed list of names we've confirmed are safe to publish. Anything else fails the build with the file, line and the exact text. Adding a name means editing the list, which shows up in the diff — so publishing a name becomes a decision someone reviewed, not an accident. The list is an **allowlist, never a denylist**. A denylist would have to contain the private names in order to match them, so the lint would itself be the leak. ## What's in here - `.github/lint/check-org-repo-literals.sh` — the lint. `git grep`s the tracked tree for org-prefixed repo literals, normalizes each name (case, a `.git` suffix, trailing periods — **not** a trailing `-`/`_`, which are legal at the end of a real slug), and fails with `file:line:match` for anything not on the allowlist. Exit `0` clean / `1` findings / `2` configuration error. `shellcheck -x` clean, works on bash 3.2 and 5. - `.github/lint/org-repo-allowlist.txt` — the list. One name per line, each with a trailing `#` comment saying why it's safe, grouped into real public repos / documented exceptions / CODEOWNERS team handles / documentation examples / Hugging Face model namespaces / test fixtures. Team handles live in their own `@`-scoped namespace, so a CODEOWNERS slug never clears a bare reference to a repo of that name. - `.github/lint/README.md` — the rule, how to add a name, why this isn't `public-repo-hygiene`, and the known limitations. - `.github/workflows/test-org-repo-literals.yml` — runs shellcheck + the lint + four smoke-test steps (**45 assertions**: exit-status, normalization, the token boundary, the team/repo namespace split, over-broad allowlist entries, the "the scan could not run" configuration errors, the `git grep` production path with a hostile `grep.*` config, and — new this round — the workflow-command escaping of a hostile tracked path, the hyphenated sibling owner, a bare-repo root, `GIT_DIR` in the environment, the per-hit cost bound and the findings cap), with **no `paths:` filter**. The leak surface is the whole tree (past leaks sat in `scripts/`, `.github/dependabot.yml` and a component README), so gating this on workflow-file paths would exempt exactly the files most likely to carry one. - `AGENTS.md` / `CONTRIBUTING.md` — the convention bullet now says the rule is CI-enforced and where the allowlist lives; the Commands/Before-you-open-a-PR blocks gained the command; the "every test workflow is path-filtered" sentence was corrected, since this one deliberately isn't. ## Stated limitation **The guard only catches org-prefixed literals.** A bare repo name — a `cloud`-style word written with no `Comfy-Org/` in front of it — cannot be linted without committing a denylist that is itself the leak. Bare-name discipline remains with review and AGENTS.md. This is stated in the script header, the README and AGENTS.md as well as here. Second limitation, same place: the name class is ASCII, so a name whose tail is non-ASCII (a homoglyph dash, a long s, CJK text) is read only as far as its ASCII prefix. The scan is pinned to `LC_ALL=C` so that limit is identical on every machine instead of a function of the runner's locale — without that pin, `grep -i` folds `U+017F`/`U+212A` into `[A-Za-z]` in a UTF-8 locale and not in the POSIX one, and the same tree would pass locally and fail on CI. The class cannot simply be widened to high bytes: measured against the tracked tree, 10 literals run into a high byte and only 5 are the deliberate homoglyph fixtures — the rest are an ellipsis, a curly apostrophe and CJK text written flush against a *real* public name in prose, which widening turns into unfixable findings. Third limitation: both scan paths pass `-I`, so a tracked blob git classifies as **binary** is never read. One `.gitattributes` line can take a whole file type out of scope. Text is the only surface this lint claims, and the script header and README now say so rather than letting a green run overstate it. Fourth limitation: **only a literal written whole is caught.** An org literal assembled at run time — `printf '%s/%s' '<org>' '<name>'`, `"${ORG}/<name>"` — never matches, so it needs no allowlist edit while the run still reports its scope clean. This is not hypothetical: the smoke tests below use exactly that spelling on purpose, which makes it the house style for org literals in this repo's workflow files. It stays fully human-readable in the source, so review is what catches it. Fifth limitation: **a left boundary is not an owner-name boundary.** The org segment must start a token, which excludes `Not<org>/x` — a different owner whose name ends in ours — but `-` is legal in a GitHub owner name and is not an identifier character, so the hyphenated sibling `Not-<org>/x` still satisfies the boundary and **is** read as one of ours. That is a false positive, not a miss: the failure mode is a red CI on a reference that has nothing to do with us, never a leak. It is kept rather than fixed so the boundary class stays byte-identical to `public-repo-hygiene`'s `REPO_REF_RE` — a name must not pass one checker and fail the other — and widening it is a change to both. Both spellings are pinned by the smoke tests. Sixth limitation: **a team `@` is judged by position, not by grammar.** An `@` glued to the tail of an identifier (`user@<org>/<name>`, email-shaped) still reads as a scope, so such a literal can draw on the `@`-scoped team entries. Reaching a private name through that needs the name spelled exactly like an allowlisted team slug. `public-repo-hygiene` reads the `@` from the same position and carries the same residual. ## Judgment calls - **A new workflow rather than widening `test-workflow-pins.yml`.** Both were offered. A repo-wide guard bolted onto a workflow named for pin linting would run the Python pin suite on every docs change and read as mis-scoped; a separate check also gives a legible name in the checks list. It is named `test-org-repo-literals.yml` rather than `repo-hygiene.yml` to avoid colliding with the existing `public-repo-hygiene.yml` reusable workflow, which is a different thing. - **This is not a second copy of `public-repo-hygiene`.** That checker is the rigorous, org-wide implementation of this same default-deny idea and is what every *other* repo should adopt. This repo cannot adopt it as a caller: it is that checker's own home, so its tests and docs deliberately commit fake-private fixture names, quote internal collaboration-tool hosts, and carry ticket-style ids by convention — all three of its categories, all intentional. I measured it: running that checker over this tree reports **1232 findings**, of which **894** are ticket-style ids, **246** are org repo/team references (its own fixtures, doc examples and the names this allowlist now covers) and **92** are internal collaboration-tool links quoted in its own docs and tests. This lint is the one category this repo *can* enforce on itself today, and the script header, the README and AGENTS.md all say new detection belongs in the org-wide checker rather than here. - **The allowlist is seeded from an actual scan of `main`, not from a guess.** Every entry corresponds to a literal that is really in the tree today, and I read the surrounding context for each one before listing it. The seeding is a superset of what was originally sketched, because the org-wide checker and its fixture corpus landed here after that sketch was written. - **Case-insensitive matching, both halves.** GitHub resolves owner names case-insensitively, so `comfy-org/<name>` reaches the same repository; matching only the canonical spelling would be a one-keystroke bypass of a default-deny control. Turning this on surfaced **6 additional literals** a case-sensitive scan missed. It also removes the need to list a name twice for its casing. - **The org segment must start a token**, using `public-repo-hygiene`'s `REPO_REF_RE` left boundary verbatim rather than a boundary invented here — the two checkers then agree on what counts as a reference, including the trade that `_` reads as identifier-continuation. Without it `grep -o` reads this org's name out of the middle of a *different* owner's (`Not<org>/whatever`) and reddens CI on a reference that has nothing to do with us. `grep -E` has no lookbehind, so the boundary character is consumed by the match and dropped in the loop; the team/scope `@` is read from that position. The boundary may itself be a **colon** (`uses:`, `see:<org>/x`), which puts a fourth colon on a `file:line:match` line, so the right-anchored split detects and corrects that case — pinned by a test asserting the reported LOCATION, not just the exit status. - **An over-broad allowlist entry is a configuration error (exit 2), not a blanket clear.** One character is otherwise all it takes to neuter the control while still looking like an ordinary allowlist edit. The guard tests behaviour rather than spelling — an entry that clears **either** of two unrelated probe names is rejected — so it covers `**`, `?*`, `[a-z]*`, `[a-y]*`, `[!z]*` and `*[a-z]` as well as `*`, without depending on having enumerated the spellings. *Either*, not both: requiring both honoured any glob that missed one probe, and `[a-y]*` is a single keystroke from `[a-z]*`. The two probes differ in first character, last character and separator, so a glob anchored at any of those positions trips one of them. All seven spellings are pinned by a smoke test. - **Team handles are a separate namespace from repo names.** An `@`-prefixed allowlist entry clears only an `@`-prefixed literal, so a slug listed for a CODEOWNERS fixture does not also clear a literal reference to a private *repo* of that name. A plain entry still clears either spelling, because `@<org>/<name>` is also how npm and GitHub Packages write a package scope — `@<org>/comfy-cli` appears 81 times here as exactly that. This is the same asymmetry `public-repo-hygiene` documents. - **Test-fixture families are enumerated, not globbed.** A `secret-*` line would pre-approve an unbounded set of names on a list whose whole value is that each name was reviewed once. All 58 fixture names that actually appear in the tree are individual lines; a new one reddens CI until someone adds it, which is the intended cost. - **The scan fails closed.** `git grep`/`grep` exit 1 for "no matches" and >1 for a real error; swallowing the latter would report a clean tree because the scan never ran, and a green run reads as coverage. - **`unreviewed-merges` is allowlisted as a documented exception**, not as a public repo — it is published by decision as the SOC-2 tracker destination. Its line points at the README and `docs/callers/detect-unreviewed-merge.md`, which already publish the name, and states the *decision* rather than a visibility: "X is private" is itself the private detail, and an entry's own line is the one place this scan structurally cannot look (the entry clears it), so the allowlist header now says what belongs there. - **The per-hit work is bounded by failing closed, not by truncating.** A name past GitHub's 100-character repo-name limit *after normalization* is reported without being compared, because normalizing an unbounded match is quadratic — measured at over two minutes for one tracked line of `<org>/comfy-cli` plus 400 KB of periods, against the job's `timeout-minutes: 10`, which turns a lint into an inconclusive run. Truncate-then-compare would have been the obvious fix and is a *widening*: that same line would have normalized down onto the allowlisted `comfy-cli`. An over-long name cannot be a real repo, so reporting it can never clear one. Round 5 moved the *order*: the documented `.git`/trailing-period strips now run before the exact test, bounded by 8 bytes of headroom, because measuring first forced `<org>/<98-char-name>.git` into the over-long branch — which skips both allowlist comparisons, so the finding named a remedy that could not silence it. Past the headroom the peel is still refused rather than run, so the quadratic bound is unchanged. Two assertions cover it — a stopwatch and a verdict check — because the cheap way to pass the stopwatch alone would be to truncate and clear. - **The finding line carries a constant `unapproved: ` prefix.** A tracked path is untrusted input to the runner, and at column zero a file named `::stop-commands::x.md` is a workflow *command* that suppresses every `::error` after it. The prefix makes that unreachable; the annotation's `file=` value is percent-escaped for the same reason. ## Verification of the deny path This change's user-facing outcome is a build failure, so the premise worth falsifying is "these literals are not legitimately in use". I ran the lint against **every tracked file on `main`** rather than only the files this diff touches: it is green, so no existing legitimate reference is denied by it, and nothing had to be scrubbed out of the tree to make it pass. The five smoke tests then prove the opposite direction — that it does fail — on an unlisted name, on a lowercased org spelling, and on a wildcard allowlist entry. ## Residual - **Bare repo names are still unguarded.** A private repo name written without the org prefix passes this lint untouched, and by design always will. That half of the AGENTS.md convention remains review-enforced. If it needs machine enforcement, that is a different mechanism (a secret-backed roster comparison, not a committed denylist) and its own piece of work. - **Non-ASCII name tails are read as their ASCII prefix.** A name spelled with a homoglyph can clear on an allowlisted prefix here. Three such fixtures already live in the tree and are allowlisted by their prefix with the codepoint named in the comment. Closing this properly needs the offset-aware scan `.github/public-repo-hygiene/check_public_repo_hygiene.py` already implements; the right fix is to reuse that, not to grow this script. - **This repo is still not enrolled in `public-repo-hygiene.yml`, and this PR does not change that.** It has the reusable workflow, the checker, the tests and a caller-bump fleet, but no self-enrollment caller — the same two-step enrollment gap AGENTS.md warns about. Enrolling it would require deciding what to do about the 894 ticket-id findings and the fixture corpus measured above, which is a scoped decision rather than a lint edit. Category 3 is now covered here; categories 1 and 2 are not covered on this repo at all. - **`AGENTS.md` is over its own integrity checker's hard ceiling and this PR makes it slightly longer.** It was 305 lines before (ceiling: 200) and is 317 after — `python3 .github/agents-md-integrity/check_agents_md.py --root .` fails on it, and has been failing on it, because this repo runs that checker for consumers but not on itself. Pre-existing and out of scope here; trimming it is a real piece of work, not a drive-by. - **A git *failure* still falls through to `grep -r`.** The scan branch is now chosen from `rev-parse --is-inside-work-tree`'s output rather than its exit status, which fixes the bare-repo and `.git`-path roots, but an actual git error (`safe.directory` refusal, a broken config, no git binary) also leaves the output non-`true` and takes the fallback, which reads untracked and ignored files. The fallback does at least read `$root` — which is what the OK line claims — and the branch is unreachable in CI, where the checkout is always a work tree owned by the runner user. Distinguishing "git refused" from "not a repo" is a separate decision about whether a `safe.directory` refusal should be exit 2. - **A runner-level `core.attributesFile` is still honoured.** The four `GIT_CONFIG_*` injection channels are unset, but that restores git's *default* config search rather than disabling it, so a `core.attributesFile` in the runner's `$HOME/.gitconfig` or `/etc/gitconfig` still marks files binary and takes them out of `git grep -I`. Pointing `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` at `/dev/null` plus `GIT_CONFIG_NOSYSTEM=1` would close it, and is deliberately not done: it would also drop a runner's legitimate `safe.directory`, which makes `rev-parse` fail and silently takes the `grep -r` fallback on every run. A PR cannot write the runner's config, so this sits on the trusted side of the same boundary the checkout does. - **A newline in a tracked path is refused, not scanned, on the `grep -r` path.** The record-shape and digit checks make it exit 2 rather than report a fabrication, so such a file is never scanned there. On the git path it is scanned normally (git C-quotes the path into one record) and reported with a C-quoted location. Renaming the file is the fix; supporting it would mean NUL-delimited output on both paths. - **`$hits` still buffers the whole scan output** before the hit loop starts, and the per-hit loop stays linear in the hit count (limitation 11, now carrying the measurement: ~5.4 ms per hit past the print cap, so the job's `timeout-minutes: 10` is reached at roughly 110,000 hits in one scan). That is inherent to capturing the scan's exit status, which is what makes it fail closed; streaming would restructure that. With the per-hit work now linear and the emission capped at 200 printed findings, what remains is memory proportional to one `grep -o` output. - **Two local suites could not be run** for want of tools on this machine, and neither is touched by this diff: `scripts/area-label/tests/test_lib.sh` (needs `yq`) and `.github/coderabbit-config/tests` (needs the pinned `jsonschema`, which is installed from `requirements.txt` in CI). Both run normally in CI. - **Two short ASCII prefixes are still allowlisted as prefixes.** `comfy-type` and `mod` are the ASCII heads of `public-repo-hygiene`'s deliberate homoglyph fixtures, so a private name spelled `<org>/mod<U+2010>…` would clear on `mod`. `core-engine`, the third, moved into the `@`-scoped team namespace and no longer clears a bare repo reference at all. Reaching the remaining two needs someone to write a homoglyph on purpose, which the script's TAMPER BOUNDARY block already places outside what this lint controls; real closure is the offset-aware scan `public-repo-hygiene` implements. ## Provenance - **Authored by:** agent-work loop - **Verified:** *Rounds 1–3:* `bash .github/lint/check-org-repo-literals.sh` on the full tracked tree: 0 findings, with the allowlist file itself in scope; `shellcheck -x` on the script: clean; `actionlint` on the workflow: clean. The script re-run under bash 3.2 and bash 5, and under `LANG=C`, `C.UTF-8` and `en_US.UTF-8`: identical result in all six combinations. All four smoke-test steps replayed locally from the workflow YAML, under both bash 3.2 and bash 5: **26/26 assertions passed at that point** — an unlisted name, a lowercased org spelling, a trailing-`-`/`_` variant of an allowlisted name, a bare reference spelled like a team slug and a literal glued to a colon all exit 1; a plain allowlisted name, a `.git`+period spelling, an `@`-prefixed team slug, a reference naming nothing and an UNHYPHENATED reference to a different owner whose name ends in ours all exit 0 (the hyphenated spelling is limitation 5 above, pinned separately as a known false positive); the allowlist entries `*`, `**`, `?*`, `[a-z]*`, `[a-y]*`, `[!z]*` and `*[a-z]` each exit 2; a work tree with no tracked files, an unenterable root, a root holding only binary blobs and an unreadable allowlist each exit 2; on the `git grep` PRODUCTION path a tracked unlisted name exits 1 and a tracked allowlisted name exits 0; and both the colon-boundary case and a repo configured with hostile `grep.column`/`grep.fullName` still report the fixture's own `file:line` (`fixture.md:2`). The `grep.*` pin was confirmed load-bearing by running the same `git grep` with and without it (`f.md:2:4: …` vs `f.md:2: …`). Multibyte boundary characters (an em dash, CJK text) still produce a correctly-located finding. `python3 .github/workflow-pins/check_workflow_pins.py`: OK, 11 workflows, 0 findings. unittest suites cursor-review 194, agents-md-integrity 46, groom 365, public-repo-hygiene 165, refresh-reviewers 48, workflow-pins 265: all OK. Shell suites bump-callers 437, preflight 278, paths-contract 70, pr-derisk 63: 0 failed (paths-contract needs a `__pycache__`-free tree — a stale one from running the Python suites fails it, which is the artifact and not the code). The homoglyph measurement quoted above was taken by re-running the scan over the tracked tree with the name class widened to high bytes and diffing the matches. *Round 4* re-verified everything from scratch after 6295758: `shellcheck -x` clean; `actionlint` on the changed workflow clean; the lint green on the full tracked tree (0 findings) under `LANG=C`, `C.UTF-8` and `en_US.UTF-8`; `python3 .github/workflow-pins/check_workflow_pins.py` OK (11 workflows, 0 findings) and its 265 unittests OK. All four smoke-test steps replayed from the workflow YAML under **both bash 3.2 and bash 5**: **45/45 assertions pass in each**, 90 total, zero `::error` lines. Step 3 was additionally replayed with `id` stubbed to report uid 0: 4 of its 6 assertions still run and only the two `chmod` ones skip, against 1 of 6 before. Each fix was reproduced failing first: `::stop-commands::a,b.md` emitted a column-zero workflow command and an unescaped `file=`; `Not-<org>/<unlisted>` exited 1 while `Not<org>/<unlisted>` exited 0; `rev-parse --is-inside-work-tree` printed `false` and exited 0 for a bare repo and for a path under `.git`, both dying with the wrong message; `GIT_DIR=b/.git GIT_WORK_TREE=b git -C a grep -l -e ''` listed **b**'s files, and end to end a clean root reported a dirty repo's finding; 400 KB of trailing periods ran past two minutes (profiled to 62 s in `${hit##*:}` alone, and the two-expansion rewrite `${value%%"${value##*[!.]}"}` measured quadratic as well) and now completes in ~2 s. Cap boundary measured exactly: a 100-character name reports "not on the allowlist", a 101-character one "longer than GitHub's 100-character repo-name limit"; a 250-finding fixture prints 200 lines, reports 250 and exits 1. Edge cases re-checked green: a multibyte (em dash, CJK) boundary character, a literal `%` in a tracked path, a colon in a real directory component, and a colon boundary and a colon in the path together — all still report the correct `file:line`. *Round 5* re-verified from scratch after 3a37181: `shellcheck -x` clean; `actionlint` on the changed workflow clean; the lint green on the full tracked tree (0 findings) under bash 3.2 and bash 5; unittest suites cursor-review 194, agents-md-integrity 46, groom 365 all OK and the bump-callers shell suite 437 passed / 0 failed. All **five** smoke-test steps replayed from the workflow YAML: **63/63 assertions pass** (45 before), zero `::error` lines. Every new assertion was then replayed against the **pre-fix** script and fails there, while the four fail-closed controls pass on both, as they must. Each fix was reproduced failing first: `<org>/<98-char>.git` and `<org>/<100-char>.` both exited 1 as over-long with the allowlist skipped; `GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.attributesFile` pointing at one `*.md binary` line made the unfixed script print the clean-tree OK line and **exit 0** for a literal it never read; a tracked file named `a:bc<LF>d.md` produced `::error file=a,line=./a::… c is not on …` — a fabricated literal at a path that does not exist, with the raw path in `line=`; `git -C docs -c grep.fullName=false` printed `x.md` where `true` prints `docs/x.md`; and `<org>/foo.<org>/private-name` scanned as one match named `foo.<org>`. Performance measured directly: 2000 unapproved literals in one tracked file went **18.4 s → 11.7 s** (8.6 ms → 5.4 ms per hit past the print cap, against 2.1 s at 200 hits). *Round 6* re-verified after eb1bcbe: `shellcheck -x` clean (including the one new `# shellcheck disable=SC2254`), `actionlint` clean, the lint green on the full tracked tree under bash 3.2 and bash 5, and the other suites unchanged (cursor-review 194, agents-md-integrity 46, groom 365, bump-callers 437 passed / 0 failed). All five smoke steps replayed: **70/70 assertions pass** (63 before), and every new one was replayed against the round-5 script and fails there. Each round-6 fix was reproduced failing first: `GIT_CONFIG_PARAMETERS="'core.attributesFile=…'"` made the round-5 script print the clean-tree OK line and **exit 0**; a tracked file named `a:1:foo<LF>x.md` printed the fabricated finding `oo` at `file=a,line=1` for a path that does not exist; the allowlist entries `public-name_<org>`, `public-name<org>` and a bare `<org>` were all **accepted**; and `--root <repo>/docs` validated that repo against *this* repo's allowlist. The `git grep` C-quoting premise was checked directly (`"a:1:foo\nx.md":1:<org>/private-name` — one record, not two), and SC2254 was checked against the build CI uses: `shellcheck --include=SC2254` exits 0 on the two `$org_glob/*` patterns and fires on the new `$org_glob/)` one. - **Deviations:** none against the requested scope. The allowlist is a superset of the originally sketched seed set (seeded from a real scan instead), a name is not listed twice for its casing (matching is case-insensitive), and the CI wiring is a new minimal workflow rather than a widened `test-workflow-pins.yml` — both were offered as implementer's choice. Review round 1 changed three defaults from the first pass: fixture families are enumerated rather than globbed, team handles got their own `@`-scoped namespace, and the over-broad-entry guard tests behaviour rather than the single `*` spelling. Review round 2 changed four more: the over-broad guard rejects on **either** probe rather than both, the org segment must **start a token** (`public-repo-hygiene`'s boundary verbatim, with the colon-boundary parse fix that needs), the `grep -r` fallback fails closed on a root with nothing scannable, and an unreadable allowlist is exit 2 rather than exit 1 — plus `git grep`'s output-format config is now pinned and a fourth smoke step exercises the git production path. Review round 4 fixed seven of eight threads and answered the eighth on the thread: the claim that `[ … ] && longest=…` as a loop body's last command aborts under `set -e` is the opposite of bash's documented behaviour (a command before the final `&&` of an `&&`/`||` list is exempt from `-e`), measured on bash 3.2 and bash 5 both as `bash -e script` and in-script, with a bare `false` body as the contrast that isolates it. Two items from those threads were deliberately **not** fixed and are recorded under **Residual** instead of as follow-ups, because neither is reachable in CI: the git-failure fallback and the `$hits` buffering. Review round 5 fixed all nine threads: the length test moved after normalization (with `MAX_STRIP` bounding the peel), `GIT_CONFIG_COUNT`/`GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` joined the `GIT_DIR` unset, a non-numeric `line` field became a refused parse rather than an escaped finding, `grep.fullName` was pinned `true` instead of `false`, the allowlist loader gained two entry rejections (`/`, and a name ending in `.<org>`/`-<org>`), the per-hit head-slice fold became a glob built once from `$ORG`, and two stale comments were corrected. One was fixed only in part and is recorded rather than filed: the per-hit loop stays linear in the hit count, and neither suggested bound was taken because both trade the *true* finding count — the number a red run's summary turns on — for a truncated one; the cost is now stated as limitation 11 with its measurement, and no follow-up ticket was filed because it is not reachable under current code. Review round 6 fixed six of seven threads: `GIT_CONFIG_PARAMETERS` joined the unset (a fourth injection channel delivering the same fail-open), the record's *shape* is validated rather than just its line field, the merged-token entry guard became the complete org-suffix test rather than a sample of separators, `--root` below a work tree now resolves the scanned repo's own allowlist, two tests that were passing for the wrong reason were rewritten to pin what they claim, and the allowlist file's own header — the copy a contributor actually opens — was brought in line with the other two. The seventh was **rejected with evidence**: SC2254 does not fire on the two `$org_glob/*` case patterns (CI's required `shellcheck -x` step passed on that exact file, and `--include=SC2254` exits 0), because a pattern ending in `*` reads as intentional globbing. The mechanism is real, and the new `$org_glob/)` pattern it does fire on carries a directive. No Linear follow-up was filed this round. Everything not fixed is under **Residual** above. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:37:37 +00:00