From the CodeRabbit review of #90. --update wrote such a path into contracts/deployed.lock, and the check then
refused the line it had just written: an entry is "kind hash path" with no space in the path, which is also the
only shape the guard reads. The path is refused before anything is written.
From the CodeRabbit review of #90: the "contracts" finding explained itself as "new contract sources", while it
is given for every change under contracts/ that is not blocked, edits and deletions too. Read the other rules'
texts against the code as well: contract-locked names the lock's own guards, submodule says "removed", binary
names the image and font types it accepts, case-collision covers two new paths, dangerous-code mentions the
environment and key stores, dot-file names its exceptions.
From the CodeRabbit review of #90.
- A lockfile or a file over 2 MB is read for credentials and hidden characters, and at most 200 findings are
listed per file. Past that the report now says how many it left out, as it already did for every other file.
- The new dot-file rule leaves .gitignore, .gitkeep and .keep alone. CONTRIBUTING named only the first two.
A full read of the guard for changes that get no finding, or a lighter one than the rule promises. Each item was
reproduced before it was fixed, and has a test.
Refused for everyone:
- criss-cross (new): a branch with more than one common ancestor with main. Its diff showed nothing while merging
it took a merged fix back out (a "git merge -s ours" of an old side branch). No merge on main has that shape.
- odd-path: any character outside ASCII in a file name. A Mac folds the long s onto s, so "contract<long s>/src/..."
and ".v<long s>code/tasks.json" landed on the real files at checkout and matched no rule.
- install-script: binding.gyp. npm builds it at install time with no script in package.json.
- lockfile: an entry's name and version have to be a name and a version. A version that walks the path gave one
package another package's tarball.
- unread (new): a file the guard cannot read (over 25 MB, past 1500 files, an error) was "unscanned", which an
owner's own pull request passes. A lockfile and a text file over 2 MB are still read for credentials and hidden
characters.
- hidden-unicode: a variation selector away from an emoji, and a carriage return inside a line (a compiler ends a
"//" comment there).
- case-collision: two new names that differ only by case. A file renamed only in its case is no longer one.
- dev-machine: CLAUDE.local.md, AGENTS.override.md, .devcontainer.json, *.code-workspace, .clinerules/, .zed/,
.roo/, .kiro/ and more of their kind.
For an owner to read:
- dot-file (new): any other new file or folder whose name begins with a dot (.coderabbit.yaml, .eslintrc.js). A
.gitignore or .gitkeep is left alone.
- A file under pages/ is a page whatever it is called, so "x.test.tsx" there is not exempt as a test. LICENSES.ts is
code, LICENSE-MIT is prose.
- auth-removed knows the app's own names (validate..., check..., admit..., assert...), and a check that is
commented out counts as removed.
- new-env: process.env?.NAME, and a helper called with the name (intEnv("NAME", 5), vm.envOr("NAME", ...)).
- contract-rules: owner, paused, feeBps, setProtocolFee, written without a library.
Also: a finding never repeats a credential (a git dependency's token was quoted in the job log); the lock script
refuses a line that is not an entry in the one shape the guard reads; CONTRIBUTING says what "contracts" and
"binary" cover and lists the three new rules.
161 tests (146 before). Every merged pull request on main and all eight open ones keep their outcome.
From the third CodeRabbit review of #88, and from reviewing those fixes.
- new-host: a scheme with no host after it ("https://" + target) had no finding. It is "(computed)" now, as a
template placeholder already was. So are an empty host ("https://@" + x), an address that ends its string and
has more joined on ("https://known.org" + ".elsewhere.io"), and "//" + host. Also read: a host behind escaped
slashes, an address with no scheme, a wildcard host. The base branch's hosts are read with the same rule.
Not seen, on purpose: a scheme that is itself split ("https:" + "//" + host). A string that is only "https:" is
how the app compares protocols, in eleven places.
- dangerous-code: the same things spelled another way. Function() without new, eval reached indirectly, a timer
given its code as a string, createElement("script"), and the environment taken as one object
({ KEY } = process.env, env: process.env).
- auth-removed: a deleted file takes its checks with it, and that was never reported, since a deleted file is not
read. Now it is, unless the same check turns up again in what the pull request adds (a move).
- check-deployed-sources.sh: --update refuses a file git does not track, writes each path as git spells it from
the root (the guard compares the lock's paths letter for letter, so "./contracts/..." would have dropped the
file's protection), and removes its temporary file when it refuses. The check says so when a locked file is
present but untracked.
- CONTRIBUTING: the check turns green after the approval and a re-run of the job, as the job summary says.
146 tests. All 65 merged pull requests on main keep their outcome.
The two lists were prose written beside the rules, so they drifted: the review found the lock's check scripts
missing from "blocked" and .gitattributes missing from "needs approval". Each point now opens with the names of
the rules it describes, the same names the check prints, and workflows.test.mjs fails when a rule is missing from
the guide or sits under the wrong outcome.
- CodeQL read `includes("http://www.w3.org/1999/xhtml")` as an incomplete check of a URL. It is a search of an SVG's
text for the one namespace under which HTML works inside it. The end of the address is matched now, which says so
and finds at least as much.
- CONTRIBUTING.md said every binary that is not an image or font is blocked. Audio, video and PDF files go to a
maintainer instead, as the guard has always had it. The two lists now also name package manager configuration,
case-colliding names, removed security checks, the license and security policy and unreadable content, and point
to the RULES table as the full list.
From the second CodeRabbit review of #88.
- SVG: the check ran on single added lines, so a handler on a line of its own, a script element under a namespace
prefix, or "javascript:" got no finding at all. The whole file is now read, after undoing character
references. Also refused: a default attribute from a document type, an animation that sets a handler, HTML inside
the image. An SVG that cannot be read in full is blocked instead of set aside as "unscanned".
- Files the site serves: app/public is read the other way round. Everything in it needs an owner except what cannot
act (images, fonts, plain text), because a browser runs script in anything served as HTML or XML and the list of
such file types is long (.xht, .xsl, .rss and more had the gap the review found for .svg). Every SVG, XML or HTML
file under app/ counts, and the app manifest, whatever the letter case of the name.
- Deleting package.json or a lockfile is a "dependencies" finding. It used to pass with nothing reported.
142 tests. All 65 merged pull requests on main keep their outcome; the one that added the brand logos gains
"needs an owner" notes for its SVGs.
Fixes from the CodeRabbit and CodeQL review of #88, and from a second read of the result.
- An owner's own pull request turns review findings into notices only when an owner pushed the head commit. The
event's sender is that pusher only for a push event whose commit is still the head; for opened and reopened the
branch's activity log is asked, and when GitHub cannot say, nobody pushed it. Before, reopening a pull request
after someone else had pushed to it counted as the owner's push.
- Dependency specs: the rule now lists what a registry spec may hold instead of every way to leave the registry.
Local paths (../../x, ./x, ~/x, /x, .), tarball names, scp-style git remotes and unknown protocols were only
"review" before.
- A manifest or lockfile the guard cannot check (too large to read, nested too deep, past the file limit) is
blocked. It used to be "unscanned", which one approval lets through. Both are read whole whatever their size.
- Reports: finding details are code spans in the job summary, so a dependency or script name cannot write a link or
HTML into it. A line of the job log no longer starts with a file name, which the runner could read as a workflow
command.
- The exit code is set, not forced: a JSON report sent into a pipe was cut at 8 KB on a Mac.
- check-deployed-sources.sh: a library with edited or added files, with a state git cannot read, or with files but
no repository is drift. The lock's last line counts without its newline, and --update works on an empty lock.
- Workflows: every checkout drops its credentials, the guard job says what its two read permissions are for, and
the tests pin the guard workflow line for line.
- Not changed: a review does not start the guard. pull_request_review runs the pull request's copy of the
workflow, so it cannot be trusted to judge, and re-running the trusted job takes a write permission that no
workflow here has. The workflow header and CONTRIBUTING.md say so, and say when to re-run by hand.
136 tests (113 before), on Node 20, 22 and 24 and in a Linux container.
This is a public repository for a wallet-connected app, and until now a pull request could edit the workflow that
judged it (ci.yml runs on pull_request, from the pull request's own copy).
pr-guard runs on pull_request_target, so main's copy of the workflow and of .github/scripts/pr-guard.mjs judges every
pull request. It has a read-only token and no secrets, and it never checks out, installs or runs anything from the
pull request: the head commit is fetched as git objects and only read. The script uses Node built-ins only.
Blocked for everyone, owners included (merging takes an admin bypass): a change to any path in
contracts/deployed.lock or under contracts/lib, anything under .github, submodule changes, symlinks, lockfile entries
that are not the package's own registry tarball with a sha512 hash, new install-time scripts, dependencies from git,
URLs, folders or aliases, credentials and key files, invisible or direction-changing Unicode, binaries that are not
real images or fonts, scripted SVGs, editor, agent and git-hook configuration, package manager configuration, and
paths that collide by letter case.
Needs an owner's approval of the exact head commit (a notice on an owner's own pull request): new files under
contracts, dependency and npm script changes, on-chain addresses, wallet calls, hosts and environment variables the
repository has not used, code execution and raw HTML, removed signature, nonce or rate-limit checks, security headers
and served static files, build, deploy and migration files, the license and security policy, and content that cannot
be read by eye. A push after the approval needs a new one.
ci.yml: actions pinned to the commits their tags point to today (checkout v4.4.0, setup-node v4.4.0,
foundry-toolchain v1.9.1) and Foundry to v1.5.1, which is what "stable" resolves to; the contracts job runs the lock
check; a guard job runs the guard's 113 tests on the image the guard itself uses; a deps job verifies npm registry
signatures and audits production dependencies.
The guard takes effect once this is on main and "pr-guard" is a required status check in the branch rules.
Kevin Codex