Skip to content

classifyPath misclassifies backslash paths: core files report grantable (2 test failures at HEAD) #34

Description

@betterpelicans-bot

Bug: classifyPath misclassifies backslash paths — core files report grantable (2 test failures at HEAD)

Summary

src/grant/core-paths.js fails its own suite at HEAD (2173d23): 2 of 27 tests in
test/grant-core-paths.test.js fail
because backslash path spellings are never
normalized before core matching and containment checks. On Linux (path.sep === '/'),
a Windows-style separator is treated as an ordinary filename character, so:

  1. A core file spelled with \ separators classifies as grantable instead of core.
  2. An absolute backslash path into a different repository classifies as grantable instead of refused.

The second case defeats the module's stated invariant ("unprovable means refused"):
C:\other-repo\src\grant\index.js resolves lexically to a file inside the current
repo named literally C:\other-repo\..., which is inside the root, hence grantable.

Why this matters (the module's own threat model)

This file exists to answer "may a signed grant cover this file?" for the non-delegable
core (the gate, policy, chain, store, hooks). Its header says: "Every ambiguous answer
it gets wrong in the permissive direction hands an agent the ability to edit the thing
that stops it."
A tool-call parameter spelling the gate engine with \ separators
(e.g. from a cross-platform harness or a model that mixes separator styles) gets
grantable — a false permit on exactly the files this list protects. The failure mode
is platform-dependent: on Windows (path.sep === '\') these same inputs likely classify
correctly, which explains why CI on a single platform would not catch it.

Reproduction (no auth, no special environment)

git clone https://github.com/githubscum/lotor && cd lotor
npm install
node --test test/grant-core-paths.test.js
#   # tests 27
#   # pass 25
#   # fail 2

Direct probe against the exported classifier:

import { classifyPath } from './src/grant/core-paths.js';
classifyPath('src\\grant\\sign.js');
// => { verdict: 'grantable', reason: 'inside the root and not core',
//      relative: 'src\\grant\\sign.js' }
// expected: 'core' (test "matches mixed separators", line 68)

classifyPath('C:\\other-repo\\src\\grant\\index.js');
// => { verdict: 'grantable', reason: 'inside the root and not core', ... }
// expected: 'refused' (test line 103, "refuses a backslash absolute path into another repository")

Verified at 2173d23 on Linux x86_64→arm64, Node v22.23.1. Both failures reproduce
from a clean clone; no environment-specifics involved.

Root cause

classifyPath() normalizes with path.resolve(root, inputPath) and then compares
against the repo root using path.sep. On POSIX, path.resolve treats \ as a plain
character — it never becomes a separator — so:

  • relative = 'src\grant\sign.js': segment split on / yields one segment,
    src\grant\sign.js, which matches no CORE_DIRS/CORE_FILES entry → falls through to grantable.
  • Containment check resCmp.startsWith(rootCmp + path.sep) passes because the whole
    backslash string is lexically "inside" the root; nothing flags the foreign absolute path.

The codebase clearly intends mixed-separator tolerance — the tests demand it, and the
module already contains the machinery (fold() for case, explicit segment-wise ancestry
matching) — but no step converts \ to / before resolution/matching.

Suggested fix direction

Normalize separators on the input before path.resolve, e.g.
inputPath.replaceAll('\\', '/') guarded so it only applies when the input is not
already a valid native path with meaningful backslashes (or: attempt both readings and
take the more restrictive verdict — consistent with the file's "crying wolf is the cheap
failure" stance). Whichever shape you prefer, the two existing failing tests are exactly
the regression tests for it.

Test evidence

not ok - matches mixed separators
  Expected values to be strictly equal:
  + actual   - expected
  + 'grantable' - 'core'

not ok - refuses a backslash absolute path into another repository
  + actual   - expected
  + 'grantable' - 'refused'

Full suite at HEAD: 831 tests, 829 pass, 2 fail (both in this file's suite).

Happy to open the fix as a PR if you tell me your preferred normalization semantics
(eager input rewrite vs. dual-read restrictive-verdict).

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions