Skip to content

pi-permission-system: the Pi infrastructure read bypass is gated on a tool-name allowlist, so bash never gets it #956

Description

@gotgenes

Problem

The Pi infrastructure read bypass is gated on a tool-name allowlist, so the package's own trust declaration holds for four tools and silently lapses for bash.

src/path/pi-infrastructure-read.ts:33-35:

if (!READ_ONLY_PATH_BEARING_TOOLS.has(toolName)) {
  return false;
}

READ_ONLY_PATH_BEARING_TOOLS is {read, find, grep, ls} (src/access-intent/path-surfaces.ts:7-12). On the bash surface the tool name is bash, so the guard rejects before the containment check ever runs. The bash gate never even receives the list — describeBashExternalDirectoryGate takes (tcc, bashProgram, resolver, normalizer) (src/handlers/gates/bash-external-directory.ts:31-36), while describeExternalDirectoryGate takes infraDirs (tool-call-gate-pipeline.ts:101-119).

The result is a split that is hard to explain:

read ~/.pi/agent/npm/node_modules/@earendil-works/   → auto-allowed
bash: ls ~/.pi/agent/npm/node_modules/@earendil-works/ → prompts

Both are reads of a directory the package itself declares as infrastructure. Only the spelling differs.

This is not the relief #800 asked for. #800 was about read-only bash commands against any external directory, and it closed correctly: the operator writes external_directory_read: { "*": "allow" } and the pure-reader core does the rest. This is the narrower case where the package has already decided the path needs no prompt, and a user has to hand-write a rule to restore, on one surface, a declaration the package makes on the others.

Why the tool-name proxy is now obsolete

READ_ONLY_PATH_BEARING_TOOLS is a stand-in for "this access is a read" — the only proof available when the bypass shipped. Since #806/#807 and ADR 0013, bash carries a real per-path direction proof. selectUncoveredExternalPaths already consumes it (src/handlers/gates/external-directory-policy.ts:66-72):

for (const { path, effect } of accesses) {
  const surface = capabilitySurfaceForEffect("external_directory", effect.effect);

Each BashExternalPath carries a TokenEffect — "read" | "write" | "unproven" with an EffectSource blame. So the predicate can ask the access its direction instead of inferring it from the tool name.

Proposed change

  1. src/path/pi-infrastructure-read.ts — take an AttributedEffect instead of a tool name; admit only effect === "read". "unproven" and "retracted" (e.g. find … -delete) stay excluded by ADR 0013 §10's fail-closed rule.
  2. src/handlers/gates/external-directory.ts:49 — map the four read-only tools to "read" at the existing call site, so today's behavior is unchanged.
  3. Thread infraDirs into the bash gate (bash-external-directory.ts, tool-call-gate-pipeline.ts:101).
  4. Filter per path, not per command, inside selectUncoveredExternalPaths. A command touching both an infrastructure path and an unrelated external one must still prompt for the second. The bash gate's existing comment already notes that a whole-command bypass flattens per-path patterns.
  5. Decide what the review log records for a mixed command — some paths infrastructure_read, others decided by a rule or a human.

Item 5 is the real design work; the rest is mechanical. Blast radius looks like 3 source files and 4 test files.

Depends on

The infrastructure list currently includes bare agentDir, which covers auth.json and mcp-oauth/, and the bypass runs ahead of policy resolution so an explicit deny does not fire. Extending that to bash extends the deny-override to a surface that can compose a read into a pipeline, which is a materially different exposure from a read landing in the transcript.

So this should land after #955, not beside it.

Out of scope

Environment

  • @gotgenes/pi-permission-system 33.0.3
  • Pi 0.86.0, macOS (arm64)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions