Skip to content

fix(policy): one checked discovery policy for policy_check.sh (follow symlinks, fail closed, stay in checkout) - #383

Merged
brenpike merged 10 commits into
mainfrom
bugfix/policy-check-discovery-policy
Sep 26, 2026
Merged

brenpike merged 10 commits into
mainfrom
bugfix/policy-check-discovery-policy

Conversation

@brenpike

Copy link
Copy Markdown
Owner

Summary

Closes #377.

tools/policy_check.sh found its input files with about 25 separate find calls. They used the default -P mode, so they never descended into a symlinked directory, and most filtered with -type f, so they dropped a name-matching symlink before any read gate saw it. Some also discarded find errors. The linter could report green on files it never looked at.

This PR gives the script one discovery policy, implemented once:

  • Follow symlinks. A single engine, discover_paths, runs find -L (DISCOVERY_FIND_BASE=(-L)) and returns find's exit status through a NUL sentinel record, so a failed or truncated discovery is never read as an empty result.
  • Fail closed on anything that cannot be followed. A status-bearing classifier, discovery_gate_status, with files, dirs and raw gates, turns dangling links, non-regular files and unreadable files into findings instead of silent skips.
  • Stay inside the checkout. Every discovered path is canonicalised with realpath -m and rejected as a finding, never read, if it resolves outside the repository root. At most one containment finding is emitted per discovery call; any further escaping paths are counted.
  • One wrapper, every site. discover_checked_paths combines the engine, the containment check and the gates. CHECK 1-15, SAFETY, COMPAT and WORKFLOW-FIXTURES all route through it. CHECK 15 uses the raw gate so that its own read gate stays its type check. The old CHECK 15 private discovery helper is deleted. The only raw find calls left are the engine and two comment-marked -P negative controls.
  • Required fixture roots (SAFETY, COMPAT, WORKFLOW-FIXTURES) are gated through the dirs gate, so a missing root is a failure rather than a [SKIP].

Witnesses

  • A DISCOVERY canary runs over committed fixtures: a symlinked directory, a dangling symlink, and an escape symlink that points outside the repo. It asserts that -L descends where -P does not, that the gate works in both directions, that the wrapper composes correctly, that the containment check and its cap work, that a nonexistent root propagates a non-zero status, and that the fixtures are still symlinks in this checkout.
  • The CHECK 15 traversal canary now runs on the shared engine.
  • A new P3 pin, tests/policy/safety-discovery-policy.json, pins 18 load-bearing literals. Each was mutation-tested.
  • tests/policy/safety-tracker-ref-guard.json has a corrected description; its pinned literals are unchanged.
  • ADR-0031 records the decision, the rejected alternatives (ban symlinks; hybrid), and a containment amendment.

Output unchanged on the real tree

policy_check.sh --strict findings are identical to main: Total 29, Allowlisted 29, New 0. Checks passed is 74 / 75 (was 72 / 73): the new DISCOVERY block and the new safety fixture each add one check. Safety fixtures: 43 / 43.

Validation

  • bash tools/validate.sh --changed: escalated to the full suite because tools/** changed. rc 0, all suites pass.
  • bash tools/validate.sh --self-test: ALL PASS.

Versioning

None. Only tools/, tests/ and docs/adr/ changed. There is no plugin/ change and no bump trigger.

Local review

Local Codex review found two issues. Both are fixed:

  • A missing fixture root was skipped cleanly instead of failing (fixed in 0d98927).
  • Discovery could read files outside the checkout through a symlink. Canonical containment fixes this (fd330e4, 2e0cade, 1b0f6a9). Codex confirmed it closed on the following pass.

The final pass raised one high finding that was rejected as an already-recorded residual: containment stops the read but not the walk, so find -L still traverses an external tree before its paths are rejected. This limit was accepted when containment was chosen, and it is recorded in ADR-0031 and in the engine header. The finding's recommendation, to drop global -L, would reverse the chosen policy.

Known residuals (recorded, not fixed here)

  • Containment stops the read, not the walk (see above).
  • Only the first escaping path per discovery call is named.
  • There is no committed symlink-loop fixture; loops surface as find's non-zero status.
  • A symlinked directory that points back inside a scanned root is scanned under two path spellings.
  • The realpath-failure branch cannot be reached from find output. It is covered only by a unit probe.
  • On checkouts with core.symlinks=false, the committed symlink fixtures become text files and the canaries fail loudly, by design.
  • A presence pin cannot catch a new raw find that bypasses the wrapper.

Follow-ups

@brenpike
brenpike merged commit 1fbd606 into main Sep 26, 2026
1 check passed
@brenpike
brenpike deleted the bugfix/policy-check-discovery-policy branch September 26, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

policy_check.sh discovery silently skips symlinked directories and non-regular name matches

1 participant