Skip to content

fix(disk-hygiene): engine gate over-gates a consumer's own engine-named script when its absolute path holds a non-keep-class characterΒ #1640

Description

@kyle-sexton

πŸ€– Agent-authored (autonomous babysit lane). Found by codex review on #1625 (P2), verified and filed rather than fixed there β€” that PR was two rounds past its fix cap and its last two fix rounds had each introduced a fresh defect, so the disciplined move was to stop patching and record this properly.

Problem

_engine_gate_relevant's unparsable branch derives marker tokens as maximal runs of path-legal characters (_MARKER_TOKEN_SPLIT = [^A-Za-z0-9._\-/\\:]+) and then requires the "provably a DIFFERENT file" escape to see an absolute path. Those two rules fight each other: any character outside the keep class splits an absolute consumer path into fragments, and the fragment carrying the filename is no longer absolute, so the escape cannot fire and the guard gates a consumer's own tool.

This is the same bug class as #1611 β€” the guard denying unrelated work β€” in a narrower population.

Reproduction

A consumer's own hygiene.py under an absolute parent path, in an operator-carrying command: python3 <abs>/hygiene.py --help && echo done.

Consumer parent directory pre-#1625 at b90084bc
consumer+tools defer GATE
consumer@tools defer GATE
consumer tools (space) GATE GATE (unchanged)
plain_tools under a Windows 8.3 temp path defer GATE

Note the last row: codex reported this for punctuation like +, @, =, but it is broader. That path contains no punctuation of its own β€” it over-gates because the enclosing Windows temp path is C:\Users\KYLESE~1\..., and ~ is outside the keep class. Short-name path segments are ordinary on Windows, so the affected population is wider than "unusual characters in a path".

Severity is bounded: this is over-gating, the fail-closed direction. It cannot cause the guard to miss an engine invocation. The cost is a consumer being denied work on their own unrelated file.

Why the obvious fix is not obvious

Widening the keep class walks straight back into the failure that started #1625's review chain. The first cut there split on shell metacharacters, and codex's first P1 showed that an assignment (engine=hygiene.py) glues with =, which is not a metacharacter β€” a real fail-open. Enumerating "safe" path punctuation re-opens exactly that door, because every character added to the keep class is a character that can also glue the marker to a neighbour.

The tension is structural: detection wants aggressive splitting, resolution wants whole paths, and one token list is currently serving both.

Suggested direction

Separate the two, rather than trying to find one delimiter set that satisfies both:

  • Detect the marker with the existing fine-grained tokens β€” unchanged, so no fail-open reopens.
  • Resolve the "provably a different file" question against the coarse whitespace/quote-delimited token that contains the fine token, which preserves the whole path.

Sketch, not prescriptive:

raw_tokens = [t.strip("'\"") for t in _LINE_CONTINUATION.sub("", command).split()]

def _provably_other(token: str) -> bool:
    candidates = [token] + [raw for raw in raw_tokens if token in raw]
    return any(
        os.path.isabs(c) and _script_path_key(c) is not None and not _samefile(c)
        for c in candidates
    )

I traced this against the shapes #1625's review chain established, and it appears to hold β€” cd <scripts>;./hygiene.py scan still gates (the coarse token <scripts>;./hygiene.py does not resolve), cd <scripts> && ./hygiene.py apply still gates (the coarse token is relative), and /tmp/consumer+tools/hygiene.py defers again. Treat that as a hypothesis, not a result β€” I did not implement or test it, and the last two changes in this area each looked equally sound before their differential run said otherwise.

Acceptance criteria

  • A consumer's own absolute hygiene.py defers in an operator-carrying command regardless of punctuation, ~ short-name segments, or spaces in its parent path.
  • Every shape in fix(disk-hygiene): match the engine marker on a filename boundaryΒ #1625's four differential matrices holds its verdict β€” in particular the assignment (engine=hygiene.py && python3 "$engine" apply), line-continuation, and post-cd relative shapes must still gate.
  • Verified as a differential against the current guard, not by reasoning alone. #1625 has the probe scripts' shape in its thread evidence if useful.

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

    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.automatedOpened by automation.needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: highSignificant impact, or blocks an imminent release; staff this cycle.status: needs-decisionAwaiting a human or maintainer judgment call.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions