fix(ci): make grep gates and node-tool gates fail closed - #93
Merged
Conversation
This repo asserts enforcement in prose and never tested that the assertion
was true. Two independent instances, both hand-verified:
1. `boundary-check.sh` rule 2 — the rule enforcing charter invariants 1 and 3
— used a PCRE negative lookahead `(?!api/)` under `grep -E`. BSD grep
rejects it ("repetition-operator operand invalid") and exits 2. `2>/dev/null`
deleted the diagnostic and `if` read rc=2 as false, so the rule printed
"✓ ring boundaries intact" without ever executing. A planted violation
passed green. CI is macos-latest, so this was dead in CI too.
2. `.claude/hooks/hcs-hook` was described as a hook that decides things. It
delegates to the Phase-0b measurement CLI, which has three `allow` paths
and no `deny` literal at all. It recorded telemetry; it never enforced.
Those are not two bugs, they are one pattern.
grep has three exit codes and the codebase treated it as having two:
0 matched, 1 clean, 2 ERROR. Every gate written as
`if grep ...; then err; fi` collapses 2 into "clean".
Changes:
- NEW `scripts/ci/lib/grep-gate.sh` — `grep_gate` surfaces rc>=2 as an
execution failure with the failing argv and grep's own diagnostic;
`gate_forbid` fails on match AND on error, passing only on a scan that
actually ran. Fixes the defect class, not the two instances found.
- `boundary-check.sh` — rule 2 rewritten as two ERE-safe stages (match deep
kernel imports, subtract `/api/` in bash) so it cannot regress to PCRE.
Rules 3/4 route through `gate_forbid`. Rule 5's `| grep -v` pipeline gets
the same two-stage treatment; the filter's exit status was masking stage 1.
- `no-live-secrets.sh` — the private-key pattern starts with `-`, so grep
parsed it as an option bundle and exited 2: that check had never fired.
Fixed with `-e`. `gitleaks | tail` meant `$?` was tail's status, making the
"gitleaks detected secrets" branch unreachable — status is now captured
before the pipe. Missing gitleaks now FAILS instead of silently substituting
the weaker regex fallback (opt in with HCS_ALLOW_WEAK_SECRET_SCAN=1).
Secret-pattern greps gain an explicit `.` path operand — `grep -r` with no
path is a GNU/BSD portability trap.
- `.mise.toml` — add gitleaks. CI provisions solely through mise-action, so
its absence meant CI had been taking the weak fallback on every run.
- `justfile` — format-check/lint/typecheck/test hard-fail when node_modules
is absent instead of echoing "(not installed yet)" and exiting 0. A fresh
clone or new worktree could produce a fully green `just verify` with zero
checks executed. `schema-drift.sh` already hard-failed on this same
condition, so two gates in one run disagreed.
- `justfile` — scoped test targets are DISCOVERED from packages/*/tests
instead of a hardcoded `case` accepting only "schemas". `just test kernel`
starts working the moment that package grows tests, with no justfile edit,
which removes a guaranteed merge collision. `--passWithNoTests` dropped:
36 test files exist, so a zero-match run is a bug (regression trap #60).
- `.claude/settings.json` — remove the Phase-0b PreToolUse registration. The
campaign was closed 2026-04-26 and kept collecting for 90 days past its own
closeout. `CLAUDE.md` corrected to stop describing it as enforcement.
Validation (executed, full transcript in the PR body):
baseline -> GREEN
planted violation -> RED (boundary-check FAILED, verify exit 1)
node_modules away -> RED (all four gates refuse to report a pass)
restored -> GREEN
Both fixtures run python3 inside an isolated environment (env -i plus a temp HOME) that intentionally lacks the managed mise trust store, and the first invocation starts with cwd at the repo root. Invoking a mise *shim* there makes mise re-resolve python3 and reject the checkout's .mise.toml before the fixture reaches its synthetic trust step, so the fixture failed for a reason unrelated to what it measures. Resolve the real interpreter in the normal environment via sys.executable, and refuse a shim path outright rather than proceeding with one. Harness-only: no change to the isolated HOME/state contract, to trusted_config_paths, or to any mise trust store. Separate commit because the D-082 ledger row describes its change set as docs-only, and these two files are neither docs nor part of that relay. Trap candidate per AGENTS.md §Update policy: "mise shim re-resolves under env -i and rejects the checkout config before the fixture's trust step."
Three required reviewers ran concurrently (hcs-architect, hcs-security-reviewer, hcs-hook-integrator) and returned blocking objections. This commit folds them in. SECURITY DEFECT INTRODUCED BY THE PREVIOUS COMMIT, NOW REMOVED The fallback secret scan was changed to `grep -rE` (prints matching lines) and then printed those lines to stderr. On a real hit that writes the credential verbatim into the terminal, the agent transcript, and the CI log of a public repository — AGENTS.md §Hard boundaries, and regression trap #18. The fallback is deleted rather than repaired. It was never a working second line of defense: - its private-key pattern began with `-`, so grep parsed it as options and exited 2 — the check had never once executed; - its remaining patterns use `\s`, a GNU extension absent from POSIX ERE, so they cannot match under the BSD grep on the macos-latest runner; - it walked node_modules/ and .git/ (where a CI checkout stores a credential header); - the HCS_ALLOW_WEAK_SECRET_SCAN escape hatch it introduced was reachable by adding one line to `.mise.toml [env]`, which `.claude/settings.json` permits an agent to edit with no prompt. gitleaks is now required with no degraded path. A scan that cannot run fails. CHARTER INVARIANT 11 — gitleaks verb `gitleaks 8.30.1 --help` lists: completion, dir, git, help, stdin, version. `detect` is no longer advertised. Switched to `gitleaks git`, per invariant 11 (no deprecated syntax where a modern replacement exists) with the installed runtime as authority per invariant 14. `git` mode also scans committed history only, so it inherently skips node_modules/ and the 2 GB .logs/ tree. FAIL-OPEN PATHS INSIDE THE FAIL-CLOSED HELPER - `grep_gate`'s two `mktemp` calls had no guard. Call sites invoke it as `out="$(grep_gate ...)" || rc=$?`, and bash suppresses errexit on the left of `||`, so an unwritable TMPDIR produced rc=1 — which the helper's own contract defines as "clean". Both now `return 2`. - Stage-2 filters used `| grep -v X || true`, which collapses the filter's rc=2 into rc=1 and silently discards a confirmed stage-1 hit — the same defect one layer down. Replaced with `gate_filter`, which filters in bash, so there is no second grep whose status can be lost and no dependency on `grep -v "a\|b"` BRE alternation (another GNU-only construct). BOUNDARY-CHECK CORRECTNESS - Rule 2 subtracted only `@hcs/kernel/api/` while matching `@hcs/kernel(/src)?/`, so `@hcs/kernel/src/api/` — the charter's own declared public surface at charter:84 — was reported as a violation. - Rule 2 now scans `packages/dashboard` as well. It is Ring 2 per the script's own header, but the guard was `[ -d packages/adapters ]` only, so dashboard→kernel-private was unenforced entirely. - Rule 2 now matches relative traversal (`../../kernel/src/x`) and bare `require`/dynamic `import`, not just the `from "@hcs/..."` alias form. The alias form is the less likely one before tsconfig path aliases are wired. - Rule 3 forbade ALL kernel→dashboard imports, contradicting charter:87, which permits `packages/dashboard/src/contracts/`. It would have made the charter's permitted path unbuildable at Ring-1 start. GITLEAKS ALLOWLIST IS NOW STRUCTURAL The first CI run with gitleaks actually installed failed — correctly. The finding is a SHA-256 content digest in a docs table, already allowlisted by `.gitleaksignore`. But that file keys on a `commit:file:rule:line` fingerprint, and this repo squash-merges (every merge rewrites the SHA) while CI checks out at depth 1 (one synthetic commit). The fingerprint could never match in CI. Replaced with `.gitleaks.toml` keyed on content shape. Verified: clean on full history AND on a depth-1 clone, while a planted AWS-shaped key still trips it. SCOPE PULLED BACK The hook-registration removal is reverted out of this PR. hcs-hook-integrator showed it was half a change: `.codex/hooks.json` still registers the same always-allow hook, so `CLAUDE.md`'s "no PreToolUse hook is registered" was false repo-wide. It also needs a DECISIONS row superseding D-047 and updates to four docs. It gets its own PR. hcs-security-reviewer also established that the framing was wrong in a way that matters: a PreToolUse hook returning `permissionDecision: "allow"` on matcher `Bash` is an unconditional auto-approve, which would bypass the deny list in `.claude/settings.json`. Removing it is a net security gain, not neutral cleanup — and that needs a fixture, not a rationale. Also: pin gitleaks to 8.30.1 rather than "latest" (unreviewed supply-chain surface; unpinned scanner means gate semantics drift). README quick start gains `npm install`, which the fail-closed gates now require. DEFERRED, EXPLICITLY: forbidden-string-scan.sh, no-runtime-state-in-repo.sh, agent-contract-identity-scan.sh, shared-state-naming-scan.sh, doc-pointer-check.sh, policy-lint.sh and shellcheck-scan.sh remain on the old shape. forbidden-string-scan carries invariants 3, 4, 5 and 11 and should be converted first in the follow-up.
This was referenced Jul 25, 2026
verlyn13
added a commit
that referenced
this pull request
Jul 25, 2026
First non-.gitkeep file in Ring 1. packages/kernel has held an empty marker
since the repo was scaffolded.
Three files: a package manifest, an empty public-API barrel, and a test suite
that guards the ring boundary.
THE EXPORTS MAP IS THE POINT
charter §Package boundary enforcement:
"packages/adapters/** cannot import from packages/kernel/src/** except
through the declared public API surface (packages/kernel/src/api/)."
That rule now has two independent enforcers:
1. scripts/ci/boundary-check.sh rule 2 — a grep. It went three months
without executing once: a PCRE lookahead under `grep -E` exited rc=2 and
`2>/dev/null` ate the diagnostic, so it printed "ring boundaries intact"
while a planted violation passed. Repaired in #93.
2. This package's `exports` map, which publishes `.` and `./api` and nothing
else.
The second is the durable one. A grep can regress silently; a missing exports
entry cannot. Verified on this host:
$ node -e "import('@hcs/kernel/api')"
RESOLVED, exports: []
$ node -e "import('@hcs/kernel/src/policy/rule-resolution')"
BLOCKED by exports map: ERR_PACKAGE_PATH_NOT_EXPORTED
Node refuses the deep import before any CI gate has an opinion. The test suite
asserts the map publishes exactly two entry points, routes both through
src/api/, and contains no wildcard or `src` path — so a future PR cannot widen
it to make a convenient import work without turning the suite red.
WHY THE BARREL IS EMPTY
Deliberate. This is the workspace scaffold, and it is separated from the first
service so that workspace wiring problems surface in a three-file diff rather
than inside a six-hundred-line one.
The first service to land here is the read-only policy-snapshot loader,
assigned by ADR 0060 §Ring-1 policy/gateway loader, with its checkpoint-level
test obligation specified by ADR 0061 — reject at the digest-verification step,
not merely at the final Decision. Both are accepted; no new ADR is required.
NO JUSTFILE EDIT WAS NEEDED
`just test kernel` works the moment this package has a tests/ directory,
because #93 replaced the hardcoded `case "$target" in ""|schemas)` with
discovery over packages/*/tests. Confirmed:
$ just test kernel
✓ packages/kernel/tests/api-surface.test.ts (5 tests)
Test Files 1 passed (1)
That was the stated payoff of the discovery change and it holds. It also means
this PR adds zero merge-collision surface on the justfile.
Class D — kernel, read path. Registers no capability, exposes no agent-callable
surface, emits no OperationShape, mints and consumes no ApprovalGrant, adds no
dependency.
Validation: `just verify` green. `just test kernel` 5/5.
8 tasks
verlyn13
added a commit
that referenced
this pull request
Jul 25, 2026
The first invocable surface this project has produced.
$ just cli policy status
snapshot .../policies/generated-snapshot/tiers.yaml
status loaded
digest sha256:7e30b768... (observed, not verified — see ADR 0079)
schema policy_rule_schema_version 0.1.0
rules 8
OPERATION CLASS TIER APPROVAL PATH CEILING
read_only_diagnostic read-safe false false not_applicable
worktree_mutation write-project true true PT24H
destructive_git write-destructive true true PT1H
...
THE CITATION RULE WORKED PROSPECTIVELY
No ADR authorized a CLI surface. ADR 0003 covers stdio + Streamable HTTP
transports. ADR 0079 §Out of scope excludes "any consumer of the returned
rules" — which a verb rendering those rules is.
Citing either would have been trap #61's third occurrence. Checking first
caught it before a false citation was written, which is the first time that
rule has prevented rather than diagnosed. ADR 0080 ships here with the code.
ADR 0079's PRECONDITION, DISCHARGED
ADR 0079 §Out of scope: "before an adapter forwards a path argument, the public
form must become kernel-resolved." So `@hcs/kernel/api` now exports
`loadBoundPolicyRules()`, which takes no path and cannot be pointed at an
arbitrary file. `resolveBoundSnapshotPath()` reads HCS_ROOT and falls back to
the kernel module's own location — charter inv. 15 warns GUI apps, launchd
jobs, and IDE extensions do not inherit shell env, so a resolver that only read
the variable would fail in exactly those contexts. The parameterized form stays
for tests, which must stage mutated snapshots.
TWO GATE BUGS, FOUND BY HAVING A SUBJECT
boundary-check rule 2 is `[ -d packages/adapters ]`-guarded and scans adapters
only. Until this package existed it had no subject. Its first real one exposed
two false positives in the repair that landed in #93:
1. The subtraction required `/api/` with a trailing slash, so it matched
`@hcs/kernel/api/policy` but NOT `@hcs/kernel/api` — the exact path
charter:84 declares legal. The rule rejected the correct import.
2. Stage 1 matched any occurrence of the token, so it then flagged the
adapter's own documentation, where the path appears in backticks.
Fixed by anchoring the subtraction on quote/slash/end and requiring stage 1 to
match a QUOTED specifier — every import form quotes it; prose does not.
Negative-controlled both ways: a planted `@hcs/kernel/src/policy/...` import and
a planted `../../../kernel/src/policy/...` traversal both still go red.
Separately, the justfile's scoped-test discovery globbed `packages/*/tests` and
missed nested packages, so `just test cli` could not find
`packages/adapters/cli/tests`. Now covers `packages/*/*/tests` too; the
unknown-target error lists `kernel`, `schemas`, `cli`.
READ-ONLY BY CONSTRUCTION
Registers no capability, emits no OperationShape, mints and consumes no
ApprovalGrant, spawns nothing, writes nothing. Dispatch is an exhaustive match
over a closed verb list, not a lookup with a default handler: unknown verb exits
2, rejected snapshot exits 1. `run(argv)` returns {exitCode, lines} so the verb
is testable without spawning a process or capturing streams.
The adapter decides nothing (inv. 1). It formats the loader's result and does
nothing else. A guard derived from policyRuleTierSchema.options asserts no tier
literal appears in adapter source, and a second asserts every @hcs/kernel import
is exactly `@hcs/kernel/api`.
The digest is labelled "observed, not verified" in the output, because ADR 0079
cut provenance verification from the loader's scope and the verb must not imply
a guarantee the kernel does not make. A test asserts that label is present.
The kernel surface lock fired again when the barrel widened — third time it has
caught an unannounced API change.
Class E — adapter read path.
Validation: `just verify` green. `just test cli` 8/8, `just test kernel` 29/29,
full suite 556.
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.
Why
This repo asserts enforcement in prose and does not test that the assertion is true. Two independent instances, both hand-verified:
boundary-check.sh:46— the rule enforcing charter invariants 1 and 3 — used a PCRE negative lookahead(?!api/)undergrep -E. BSD grep rejects it and exits 2.2>/dev/nulldeleted the diagnostic;ifread rc=2 as false. The rule printed✓ ring boundaries intactwithout ever executing. A planted violation passed green. CI runsmacos-latest, so it was dead in CI too..claude/hooks/hcs-hookis described as a hook that decides things. It delegates to the Phase-0b measurement CLI, which has three exit-0 paths and nodeny. It recorded telemetry; it never enforced.Not two bugs — one pattern.
grephas three exit codes and the codebase treated it as having two:0matched,1clean,2error. Every gate written asif grep …; then err; ficollapses2into "clean".Change class
J — enforcement tooling. Objections required per
IMPLEMENT.md:131/132/136.All three reviewers ran and returned blocking objections. Their findings are folded into commit 3. Summary below.
What this PR does
New —
scripts/ci/lib/grep-gate.sh.grep_gatesurfacesrc>=2as an execution failure with the failing argv and grep's diagnostic.gate_forbidfails on match and on error.gate_filterdoes stage-2 subtraction in bash so no second grep's exit status can be lost. Fixes the class, not the instances.boundary-check.shpackages/dashboardtoo (it is Ring 2 but was never scanned), and matches relative traversal +require/dynamicimport, not only the@hcs/alias form. Rule 3 now honors charter:87'spackages/dashboard/src/contracts/exception instead of forbidding it. Rule 5's| grep -vpipeline no longer masks stage 1.no-live-secrets.shdetect→gitper invariant 11. Status captured before the pipe (the failure branch was unreachable)..gitleaks.toml(new).gitleaksignore..mise.tomlgitleaks = "8.30.1", pinned.justfilenode_modules. Scoped test targets discovered frompackages/*/tests.--passWithNoTestsdropped (trap #60).README.mdnpm install, now required.Reviewer round — what changed
hcs-security-reviewer found a defect I introduced. The fallback scan used
grep -rE(prints matching lines) and echoed them to stderr — on a real hit that writes the credential into the CI log of a public repo.AGENTS.md§Hard boundaries; regression trap #18.Rather than repair it, the fallback is deleted. It was never a working second line of defense: its private-key pattern began with
-so grep parsed it as options (never executed); its other patterns use\s, absent from POSIX ERE, so they cannot match under BSD grep on the CI runner; it walkednode_modules/and.git/; and itsHCS_ALLOW_WEAK_SECRET_SCANhatch was reachable by one line in.mise.toml [env], which settings permit an agent to edit unprompted.Fail-open paths inside the fail-closed helper.
grep_gate'smktempcalls had no guard — errexit is suppressed on the left of||, so an unwritableTMPDIRproduced rc=1, i.e. "clean". Stage-2| grep -v X || truecollapsed the filter's rc=2 the same way. Both fixed.Two correctness bugs in
boundary-check. Rule 2 subtracted only@hcs/kernel/api/while matching@hcs/kernel(/src)?/, so the charter's own public path@hcs/kernel/src/api/was flagged as a violation. Rule 3 forbade all kernel→dashboard imports, contradicting charter:87.Hook removal pulled out of this PR.
hcs-hook-integratorshowed it was half a change —.codex/hooks.jsonstill registers the same always-allow hook, soCLAUDE.md's "no PreToolUse hook is registered" was false repo-wide. It also needs aDECISIONSrow superseding D-047 and four doc updates. Its own PR.hcs-security-revieweradditionally established the framing was wrong in a way that matters: a PreToolUse hook returningpermissionDecision: "allow"on matcherBashis an unconditional auto-approve that would bypass the deny list. Removing it is a net security gain, not neutral cleanup — and that claim needs a fixture, not a rationale.The CI failure was the gate working
The first CI run failed. gitleaks had never run in CI (absent from
.mise.toml), so adding it turned the previously-unreachable failure branch on, and it fired immediately.The finding is a SHA-256 content digest in a docs table — already allowlisted by
.gitleaksignore. But that file keys on acommit:file:rule:linefingerprint, and this repo squash-merges (every merge rewrites the SHA) while CI checks out at depth 1 (one synthetic commit). The fingerprint could never match in CI. Reproduced locally with a depth-1 clone.Replaced with
.gitleaks.tomlkeyed on content shape. Verified: clean on full history and on a depth-1 clone, while a planted AWS-shaped key still trips it.Validation — executed
@hcs/kernel/src/api/../../kernel/src/xTMPDIRGATE ERROR — mktemp failedPATHnode_modulesmoved asidejust test kernelNo planted fixture was ever committed (
charter:84).Boundary checks
snapshot-binding-checkallowlist and digest checkpoints untouchedDeferred, explicitly
forbidden-string-scan.sh,no-runtime-state-in-repo.sh,agent-contract-identity-scan.sh,shared-state-naming-scan.sh,doc-pointer-check.sh,policy-lint.sh,shellcheck-scan.shremain on the old shape.forbidden-string-scancarries invariants 3, 4, 5 and 11 and should be converted first.Also deferred to PR-2b: the negative-control harness (one planted fixture per gate, where a gate that passes its own negative control fails the build). This PR is proven by the manual tripwire above; 2b makes that proof automatic. Both reviewers flagged that rules 2/3/5 have zero input today while
packages/kernelandpackages/adaptersare empty — which is exactly the condition that produced the original bug.