Skip to content

[oss-candidate] @actions/glob: honor explicit hashFiles roots outside the workspace - #1

Closed
askalf wants to merge 2 commits into
mainfrom
fix/glob-explicit-roots
Closed

askalf wants to merge 2 commits into
mainfrom
fix/glob-explicit-roots

Conversation

@askalf

@askalf askalf commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • @actions/glob's hashFiles() discards any entry of an explicit roots list that does not resolve under GITHUB_WORKSPACE, unless the unrelated allowFilesOutsideWorkspace flag is also set. The discard itself is logged at core.debug only.
  • Consequence: a caller who wants both the containment that roots exists to provide and a root outside the workspace — an action hashing its own files (GITHUB_ACTION_PATH) alongside the workspace is the motivating case — has no way to express it. The declared root is dropped and the hash covers only the workspace half. For a cache key that means a permanent miss, or a key shared by two genuinely different inputs.
  • The drop is warned about — a core.warning names each skipped file — but the only remedy that warning suggests, allowFilesOutsideWorkspace: true, disables the root check for every matched file, so the caller gets a wider allowlist than they asked for. Correct containment plus an outside root is unreachable on base.
  • Fix: one guard. An explicit roots list is the allowlist, so the workspace restriction only applies when the caller did not supply one. allowFilesOutsideWorkspace keeps its documented meaning for the default-roots case and for the per-file check.
  • Introduced by @actions/glob: extend hashFiles options actions/toolkit#2357 (@actions/glob: extend hashFiles options, merged 2026-07-14), which added roots and this restriction together. 8 lines changed in src; 5 regression tests (3 discriminating + 2 controls) added next to the existing roots tests.

Repro on base vs. the fix (repro.cjs, in this directory — a workspace dir, an "action" dir outside it, one file in each, both declared in roots):

$ node repro.cjs /agent-workspace/oss/toolkit-basewt      # base 193fa46c
workspace file only          : b52e08eacc44de13
roots: [ws, action]          : b52e08eacc44de13
roots: [ws, action] + opt-in : 0b82ee4cd1ba27a2

BUG: declared root `action` was ignored : true
     (the opt-in is the only way in)    : true

$ node repro.cjs /agent-workspace/oss/toolkit-wt-rw1789476094   # head 6b2f5c9
workspace file only          : b52e08eacc44de13
roots: [ws, action]          : 0b82ee4cd1ba27a2
roots: [ws, action] + opt-in : 0b82ee4cd1ba27a2

BUG: declared root `action` was ignored : false
     (the opt-in is the only way in)    : true

Regression tests, both arms, at this head:

$ npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit   # BASE 193fa46c, head's test file
FAIL packages/glob/__tests__/hash-files.test.ts (9.932 s)
    ✕ honors an explicit root outside the workspace without the opt-in (21 ms)
    ✕ honors explicit roots when every one of them is outside the workspace (5 ms)
    ✕ honors an explicit root that is a symlink pointing outside the workspace (16 ms)
Tests:       3 failed, 14 passed, 17 total

$ npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit   # head 6b2f5c9
PASS packages/glob/__tests__/hash-files.test.ts (9.521 s)
Tests:       17 passed, 17 total

Upstream

  • Repo: actions/toolkit, default branch main
  • Base sha: 193fa46c20fde8b0ed54194bc08b841c78c0776d
  • File: packages/glob/src/internal-hash-files.ts, function hashFiles() (the root-resolution loop, lines 115-139 on base)
  • Test file: packages/glob/__tests__/hash-files.test.ts
  • Package: @actions/glob 0.7.0

Bug

Trigger. Call hashFiles(patterns, workspace, {roots: [...]}) where at least one entry of roots is not under GITHUB_WORKSPACE, and leave allowFilesOutsideWorkspace unset (its default, false).

Wrong outcome. On base, the root-resolution loop applies the workspace restriction to every root, including explicitly supplied ones:

if (
  !allowOutside &&
  !isInResolvedRoots(resolvedRoot, [resolvedWorkspace])
) {
  writeDelegate(`Skipping root outside workspace: ${resolvedRoot}`)
  continue
}

Every file under that root then fails the later per-file isInResolvedRoots check and is skipped, and hashFiles returns a hash of the surviving subset — or '' if no root survived. Two distinct input sets therefore produce the same hash, which is precisely the property a cache key must not have.

How visible is it? The root-drop line itself is writeDelegate, i.e. core.debug unless the caller passed verbose — so why the files vanished is not in a normal Actions log. The consequence is not silent, though: the skipped files fall into outsideRootFiles and the tail of hashFiles emits a real core.warning naming each one. Measured on base, running the README-shaped probe without the opt-in:

::warning::Some matched files are outside the allowed root(s) and were skipped:%0A- /tmp/probe-action-AkKGdP/b.json -> /tmp/probe-action-AkKGdP/b.json%0ATo include them, set 'allowFilesOutsideWorkspace: true' in your options.

and when every declared root is outside the workspace, the other warning fires instead and the return value is '':

::warning::Could not resolve any allowed root(s); no files will be considered for hashing.

So the user is told. What they are told is the problem: the only remedy the warning offers over-widens the allowlist. Following it (allowFilesOutsideWorkspace: true) disables the per-file root check for every matched file, including files under neither declared root — destroying the containment the roots option exists to provide. There is no third option on base.

Blast radius. Anyone using the roots option added in actions#2357 with a directory outside the workspace and wanting containment. Both workarounds a caller would discover are wrong in the same direction: allowFilesOutsideWorkspace: true widens to all matched files, and passing a common ancestor as the root widens further. The option is new (0.7.0, 2026-07-14), which is consistent with there being no issue report yet.

Not the README example. The example in packages/glob/README.md:100-104 passes allowFilesOutsideWorkspace: true alongside roots: [GITHUB_WORKSPACE, GITHUB_ACTION_PATH], which makes !allowOutside false, so the buggy guard never fires for it. Measured on base — the README shape is immune, the same roots without the opt-in are not:

$ node readme-probe.cjs /agent-workspace/oss/toolkit-basewt      # base 193fa46c, lib explicitRoots=0
workspace only                           : d5f9d4cf11caecf4
README example shape (WITH the opt-in)   : e995b26f1ff802f9
same roots, NO opt-in                    : d5f9d4cf11caecf4

README example dropped the action root   : false
no-opt-in variant dropped the action root: true

The README example is what a caller writes when they are willing to give up containment. The bug is what happens to a caller who is not.

Repro

repro.cjs and readme-probe.cjs (both in this directory) build the same shape: a workspace dir, an "action" dir outside it, one file in each, and both dirs declared in roots. readme-probe.cjs additionally runs the README's example verbatim.

$ cd /agent-workspace/oss/toolkit-basewt && git rev-parse --short HEAD
193fa46
$ (cd packages/glob && npx tsc)
$ grep -c explicitRoots packages/glob/lib/internal-hash-files.js
0
$ node /agent-output/oss/toolkit/fix-glob-explicit-roots/repro.cjs /agent-workspace/oss/toolkit-basewt
workspace file only          : b52e08eacc44de13
roots: [ws, action]          : b52e08eacc44de13
roots: [ws, action] + opt-in : 0b82ee4cd1ba27a2

BUG: declared root `action` was ignored : true
     (the opt-in is the only way in)    : true

roots: [ws, action] returns the same hash as hashing the workspace file aloneaction.json was never hashed. The third line shows the only way to get both files in on base is the opt-in flag, which also removes the containment guarantee. The run also emits the ::warning::Some matched files are outside the allowed root(s) and were skipped line quoted under Bug.

Same script at this head:

$ cd /agent-workspace/oss/toolkit-wt-rw1789476094 && git rev-parse --short HEAD
6b2f5c9
$ grep -c explicitRoots packages/glob/lib/internal-hash-files.js
2
$ node /agent-output/oss/toolkit/fix-glob-explicit-roots/repro.cjs /agent-workspace/oss/toolkit-wt-rw1789476094
workspace file only          : b52e08eacc44de13
roots: [ws, action]          : 0b82ee4cd1ba27a2
roots: [ws, action] + opt-in : 0b82ee4cd1ba27a2

BUG: declared root `action` was ignored : false
     (the opt-in is the only way in)    : true

The declared root is honored, and it now agrees with the opt-in result for this input — i.e. both files, and only those two. The README-shaped call is unaffected by the fix (e995b26f1ff802f9 on both arms; see the readme-probe.cjs transcripts under Bug and Test evidence).

Fix

   // Resolve roots up front; warn and skip any that fail to resolve.
-  // If allowFilesOutsideWorkspace is not enabled, roots are restricted to the resolved workspace.
+  // When the caller did not specify roots, the workspace is the only allowed
+  // root and `allowFilesOutsideWorkspace` is the opt-in that widens it.
+  // An explicit `roots` list is itself the allowlist, so its entries are
+  // honored as given - otherwise a root outside the workspace would be
+  // dropped here and the files under it silently skipped.
+  const explicitRoots = options?.roots !== undefined
   const resolvedRootsSet = new Set<string>()
   const roots = options?.roots ?? [resolvedWorkspace]
 
   for (const root of roots) {
     try {
       const resolvedRoot =
         root === resolvedWorkspace ? root : fs.realpathSync(root)
 
       if (
+        !explicitRoots &&
         !allowOutside &&
         !isInResolvedRoots(resolvedRoot, [resolvedWorkspace])
       ) {

Why this is the minimal correct change. The defect is one conjunct in one predicate. explicitRoots distinguishes the two cases the code was conflating:

  • No roots given → roots defaults to [resolvedWorkspace], the workspace is the only allowed root, and allowFilesOutsideWorkspace is the documented opt-in that widens it. Unchanged. (The restriction is in fact a no-op in this branch — the sole root is the workspace — but it is left intact so the flag's meaning is not quietly altered.)
  • roots given → the caller has stated the allowlist. Honor it.

Everything downstream is untouched: the per-file isInResolvedRoots(resolvedFile, resolvedRoots) check, the allowOutside per-file branch, the outside-root warning, dedup via resolvedRootsSet, and the realpathSync failure path all behave exactly as before. Containment is not weakened — a file under no declared root is still skipped without the opt-in (pinned by control 1).

The documented contract is unchanged. After the fix, when roots are explicit, allowFilesOutsideWorkspace no longer affects root admission at all — only the per-file check. That is what the README's option text already says: "Only files that resolve under (or equal) one of these roots are hashed" and "Explicit opt-in to include files outside the specified root path(s)" — root path(s), not workspace. The prose needs no edit; only the guard did not match it.

options?.roots !== undefined rather than a truthiness test is kept because it states the intent (was the option supplied?) and is robust if the declared type ever admits null. It is not load-bearing for present behaviour, and no test can pin it: the two forms are equivalent for every value of roots?: string[], since [] is truthy in JS.

$ node -e "..."
[]           !!x = true   x !== undefined = true
undefined    !!x = false  x !== undefined = false
null         !!x = false  x !== undefined = true

(Only null distinguishes them, and ?? [resolvedWorkspace] sends null to the default either way. Mutating the guard to !!options?.roots on base leaves the suite green — see Boundaries row 3.)

Alternatives rejected.

  1. Emit a core.warning when dropping a root, keep the behaviour. A warning already exists downstream; the problem is not that the drop is unreported, it is that the documented GITHUB_ACTION_PATH usage remains impossible without over-widening.
  2. Restrict against the union of the workspace and all supplied roots. Self-referential — every supplied root trivially contains itself, so it reduces to this fix with more code.
  3. Update the README to document the flag as required alongside outside-workspace roots. That is what the README's example already does, and it is exactly the compromise this fix removes: the opt-in is all-or-nothing per file, so it makes containment unavailable to the callers who most need it. The guard contradicts the option's own documented prose; fixing the guard is the smaller and safer change.
  4. Fix @actions/cache's "@actions/glob": "^0.6.1" range at the same time. Real (see Prior art) but a separate change — one bug per PR.

Test evidence

Five tests added to packages/glob/__tests__/hash-files.test.ts — three by the hunt (9e11758) and two by the adversarial verification pass (6b2f5c9) — placed with the existing roots/allowFilesOutsideWorkspace tests and following the file's conventions (getTestTemp() for in-workspace dirs, fs.mkdtemp in os.tmpdir() for outside-workspace dirs — the suite pins GITHUB_WORKSPACE to __dirname — and io.rmRF cleanup in finally). The file holds 17 tests at this head: 12 pre-existing, 5 added.

# Test Base Head Pins
1 honors an explicit root outside the workspace without the opt-in FAIL PASS The bug, mixed inside+outside roots: the explicit outside root is honored, and exactly the two declared files are hashed (compared against the opt-in result, so it also catches over-inclusion)
2 honors explicit roots when every one of them is outside the workspace FAIL PASS The all-outside shape — no in-workspace root to mask the drop, so base returns '' outright. A distinct failure mode from test 1
3 honors an explicit root that is a symlink pointing outside the workspace FAIL PASS realpathSync resolves the root before the guard; also asserts that declaring the link target gives the same digest, pinning that containment is computed on resolved paths
4 (control) still restricts to the allowed roots when roots are explicit PASS PASS Containment is not weakened by the fix: a file under neither declared root is still skipped, and the opt-in still widens. Verified genuine: mutating the per-file check to if (false) — the over-broadening the fix must not cause — fails 4 tests including this one
5 (control) returns empty when every explicit root fails to resolve PASS PASS The realpathSync failure path: an unresolvable root yields ''. Note the scope honestly — the test globs files under a different (in-workspace) directory than the unresolvable root, so it asserts the return value, not the internals of the catch; mutating the catch to add the raw root to the set leaves it passing

Tests 4 and 5 pass on base by design and are named (control) for that reason: they exist to prove the fix does not weaken the guard it touches. Tests 1-3 each fail on base with a distinct assertion.

Fails-before — base worktree at 193fa46c with this head's test file copied in and the base src (measured: grep -c explicitRoots0 in both src and the per-arm-built lib):

$ cd /agent-workspace/oss/toolkit-basewt && npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit --verbose
FAIL packages/glob/__tests__/hash-files.test.ts (9.932 s)
  globber
    ✓ basic hashfiles test (57 ms)
    ✓ basic hashfiles no match should return empty string (5 ms)
    ✓ followSymbolicLinks defaults to true (11 ms)
    ✓ followSymbolicLinks set to true (8 ms)
    ✓ followSymbolicLinks set to false (5 ms)
    ✓ multipath test basic (11 ms)
    ✓ hashes files in allowed roots only (34 ms)
    ✓ skips outside-root matches by default (hash unchanged) (13 ms)
    ✓ allows files outside roots if opted-in (hash changes) (27 ms)
    ✓ excludes files matching exclude patterns (22 ms)
    ✓ hashes files outside GITHUB_WORKSPACE only when opted-in (25 ms)
    ✕ honors an explicit root outside the workspace without the opt-in (21 ms)
    ✓ (control) still restricts to the allowed roots when roots are explicit (20 ms)
    ✓ (control) returns empty when every explicit root fails to resolve (2 ms)
    ✕ honors explicit roots when every one of them is outside the workspace (5 ms)
    ✕ honors an explicit root that is a symlink pointing outside the workspace (16 ms)
    ✓ applies relative exclude patterns across all allowed roots (24 ms)

Test Suites: 1 failed, 1 total
Tests:       3 failed, 14 passed, 17 total
Time:        10.287 s

with the three assertions:

  ● globber › honors an explicit root outside the workspace without the opt-in

    expect(received).not.toEqual(expected) // deep equality

    Expected: not "d117a759564edad598c48e6ac0036d16889bb74eb94ab50a3c0a0a33bbfa5817"

      297 |       })
      298 |       expect(both).not.toEqual('')
    > 299 |       expect(both).not.toEqual(insideOnly)
          |                        ^
      300 |
      301 |       // And it hashes exactly those two files, no more.
      302 |       const expected = await hashFiles(patterns, '', {

      at packages/glob/__tests__/hash-files.test.ts:299:24

  ● globber › honors explicit roots when every one of them is outside the workspace

    expect(received).not.toEqual(expected) // deep equality

    Expected: not ""

      384 |         roots: [actionRootA, actionRootB]
      385 |       })
    > 386 |       expect(hash).not.toEqual('')
          |                        ^
      387 |
      388 |       // Reaching the same files through the opt-in yields the same digest.
      389 |       const withOptIn = await hashFiles(patterns, '', {

      at packages/glob/__tests__/hash-files.test.ts:386:24

  ● globber › honors an explicit root that is a symlink pointing outside the workspace

    expect(received).not.toEqual(expected) // deep equality

    Expected: not ""

      413 |       // the caller named it explicitly, so it is honored.
      414 |       const hash = await hashFiles(`${linkPath}/*`, '', {roots: [linkPath]})
    > 415 |       expect(hash).not.toEqual('')
          |                        ^
      416 |
      417 |       // Containment is still computed on resolved paths, so declaring the
      418 |       // link target directly produces the same digest.

      at packages/glob/__tests__/hash-files.test.ts:415:24

Test 1's failure is the sharp one: the hash with both roots declared is byte-identical to the workspace-only hash — the declared root contributed nothing. Tests 2 and 3 fail differently ('', i.e. nothing was hashable at all), which is why they are separate rows rather than variants of row 1.

Passes-after — this head (6b2f5c9, measured: grep -c explicitRoots2 in src and lib):

$ cd /agent-workspace/oss/toolkit-wt-rw1789476094 && npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit --verbose
PASS packages/glob/__tests__/hash-files.test.ts (9.521 s)
  globber
    ✓ basic hashfiles test (56 ms)
    ✓ basic hashfiles no match should return empty string (6 ms)
    ✓ followSymbolicLinks defaults to true (13 ms)
    ✓ followSymbolicLinks set to true (11 ms)
    ✓ followSymbolicLinks set to false (7 ms)
    ✓ multipath test basic (14 ms)
    ✓ hashes files in allowed roots only (32 ms)
    ✓ skips outside-root matches by default (hash unchanged) (16 ms)
    ✓ allows files outside roots if opted-in (hash changes) (35 ms)
    ✓ excludes files matching exclude patterns (25 ms)
    ✓ hashes files outside GITHUB_WORKSPACE only when opted-in (26 ms)
    ✓ honors an explicit root outside the workspace without the opt-in (29 ms)
    ✓ (control) still restricts to the allowed roots when roots are explicit (24 ms)
    ✓ (control) returns empty when every explicit root fails to resolve (2 ms)
    ✓ honors explicit roots when every one of them is outside the workspace (12 ms)
    ✓ honors an explicit root that is a symlink pointing outside the workspace (10 ms)
    ✓ applies relative exclude patterns across all allowed roots (21 ms)

Test Suites: 1 passed, 1 total
Tests:       17 passed, 17 total
Time:        10.107 s

All 12 pre-existing tests in the file are green in both arms — the fix changes nothing they assert.

Probes (not tests; they exercise lib/ via require, built per arm):

$ node readme-probe.cjs <base>   # lib explicitRoots=0
workspace only                           : d5f9d4cf11caecf4
README example shape (WITH the opt-in)   : e995b26f1ff802f9
same roots, NO opt-in                    : d5f9d4cf11caecf4
README example dropped the action root   : false
no-opt-in variant dropped the action root: true

$ node readme-probe.cjs <head>   # lib explicitRoots=2
workspace only                           : d5f9d4cf11caecf4
README example shape (WITH the opt-in)   : e995b26f1ff802f9
same roots, NO opt-in                    : e995b26f1ff802f9
README example dropped the action root   : false
no-opt-in variant dropped the action root: false

$ node empty-roots-probe.cjs <base>
::warning::Could not resolve any allowed root(s); no files will be considered for hashing.
roots: [] => ""
$ node empty-roots-probe.cjs <head>
::warning::Could not resolve any allowed root(s); no files will be considered for hashing.
roots: [] => ""

Project tooling (the three things .github/CONTRIBUTING.md requires before a PR is accepted), at this head:

$ npx prettier --check packages/glob/src/internal-hash-files.ts packages/glob/__tests__/hash-files.test.ts
Checking formatting...
All matched files use Prettier code style!
rc=0

$ npx eslint packages/glob/src/internal-hash-files.ts packages/glob/__tests__/hash-files.test.ts
rc=0

$ cd packages/glob && npx tsc --noEmit
rc=0

The repo-wide npm run format-check / npm run lint / npm test were not run whole — they cover every package in the monorepo and the change is confined to one file in one package. The commands above are those same tools scoped to the touched files.

Verification method

executed, on Linux (Alpine container), Node v24.19.0, npm 11.17.0 — which is one of the two Node versions in the upstream matrix (unit-tests.yml: node-version: [20.x, 24.x]).

Every transcript in this body was produced at head 6b2f5c90b7d369b4d6b34f82e4945a5755e68799, in a rebuild of both arms from scratch (the hunt's and the verification's worktrees no longer exist). Nothing is inherited.

A/B methodology, stated because it is the part most easily got wrong: the two arms are two separate git worktrees, not a file swapped in place — /agent-workspace/oss/toolkit-basewt detached at 193fa46c with this head's test file copied in, and /agent-workspace/oss/toolkit-wt-rw1789476094 at the PR head. node_modules and the dependency packages' lib/ are shared between arms, but packages/glob/lib is built per arm — sharing it makes both arms read the same compiled output and prints a convincing false negative for any require-based probe. Nothing was ever staged or restored in the candidate worktree to produce the base numbers, so the committed diff cannot have been polluted by the evidence gathering.

Arm integrity was measured rather than assumed, before every transcript:

$ grep -c explicitRoots <base>/packages/glob/src/internal-hash-files.ts   -> 0
$ grep -c explicitRoots <base>/packages/glob/lib/internal-hash-files.js   -> 0
$ grep -c explicitRoots <head>/packages/glob/src/internal-hash-files.ts   -> 2
$ grep -c explicitRoots <head>/packages/glob/lib/internal-hash-files.js   -> 2

Head integrity:

$ git diff HEAD --stat                                                  # empty
$ git diff 193fa46c20fde8b0ed54194bc08b841c78c0776d..HEAD --stat
 packages/glob/__tests__/hash-files.test.ts | 156 +++++++++++++++++++++++++++++
 packages/glob/src/internal-hash-files.ts   |   8 +-
 2 files changed, 163 insertions(+), 1 deletion(-)

Adversarial pass. An independent run rebuilt the boundaries ledger from the diff, added tests 2 and 3 (rows 11 and 13, both failing on base), and mutation-tested both controls and the !== undefined guard. Its findings are recorded in this body: control 4 is genuine, control 5's scope is narrower than first claimed (row 10), and the !== undefined/truthiness distinction is not pinnable (row 3). The full write-up is at #1 (comment).

Not covered here, for the reviewer: Windows and macOS. isInResolvedRoots has a process.platform === 'win32' lowercasing branch, and the fix's guard sits before that call, so it is platform-independent by construction — but the assertion that the fixed path behaves identically on Windows is not executed evidence from any run. The upstream matrix (ubuntu-latest, macos-latest-large, windows-latest × Node 20/24) covers it. Note the base already had no Windows-specific test for the roots option.

Fork CI: none. GitHub Actions has never been enabled on askalf/toolkit (the fork was created during the hunt; enabling it is a manual click with no API). gh pr checks 1 --repo askalf/toolkit reports no checks — an absence of CI, not a failing CI. So there is no fork CI run to link, and the Windows/macOS legs are confirmed by neither this run nor the fork — only by upstream CI once submitted.

Prior art

$ gh search prs --repo actions/toolkit "allowFilesOutsideWorkspace" --limit 20
actions/toolkit  2357  merged  @actions/glob: extend hashFiles options   2026-07-14T12:57:05Z

$ gh search prs --repo actions/toolkit "hashFiles roots" --limit 20
(no results)

$ gh search prs --repo actions/toolkit "internal-hash-files" --limit 20
actions/toolkit  975   open    Keep order when hashFiles when globbing multiple files   2022-07-23T05:59:33Z
actions/toolkit  2357  merged  @actions/glob: extend hashFiles options                  2026-07-14T12:57:05Z

$ gh search issues --repo actions/toolkit "allowFilesOutsideWorkspace" --limit 20
(no results)

$ gh search issues --repo actions/toolkit "hashFiles roots" --limit 20
(no results)

$ gh pr list --repo actions/toolkit --state open --limit 100 --json number,title \
    --jq '.[]|select(.title|test("glob|hash";"i"))|"\(.number) \(.title)"'
2465 chore(deps): bump brace-expansion from 5.0.7 to 5.0.9 in /packages/glob
2204 Fix glob pattern matching on Windows by normalizing path separators
2107 improve glob performance by not ignoring negate

No open or closed PR addresses this. No issue reports it — consistent with a new option whose failure mode surfaces as "my cache never hits" rather than as an error.

On the issue this hunt started from (actions#2484). Worth recording because it changes what a maintainer should do with it: actions#2484's stated root cause — minimatch@3's try { return require('path') } being left intact by @rollup/plugin-commonjs (ignoreTryCatch defaults to true), so sep falls back to / in an ESM bundle — is already fixed on main. packages/glob is at 0.7.0 with minimatch: ^10.2.5 (bumped in 6bd5e50), and minimatch 10 derives sep from process.platform with no require() in the path. The issue's own "Suggested resolution" says as much.

What is still live from actions#2484 is its point 1: packages/cache/package.json declares "@actions/glob": "^0.6.1", a range that will not select 0.7.0, so @actions/cache@6.2.0 consumers still get the bundling-unsafe version. That is a dependency-range change in a different package, not a code bug in packages/glob — it is recorded as a separate follow-up and is deliberately not in this PR.

Policy

.github/CONTRIBUTING.md, "Development Life Cycle":

Note that before a PR will be accepted, you must ensure:

  • all tests are passing
  • npm run format reports no issues
  • npm run lint reports no issues

All three satisfied for the touched files (transcripts under Test evidence). The same file's "Enhancements and Feature Requests" section asks for a feature request and an ADR before significant effort — that governs enhancements; this is a bug fix with a failing regression test, filed under "Issues … for both bugs and enhancement requests".

AI stance: silent. No AI/LLM/agent policy exists in this repo. The full recursive tree of main was dumped (gh api repos/actions/toolkit/git/trees/main?recursive=1) and pattern-matched for AI|LLM|POLICY|CONDUCT|CONTRIB|GUIDELINE; the only matches are .github/CONTRIBUTING.md, CODE_OF_CONDUCT.md, .github/ISSUE_TEMPLATE/{bug_report,enhancement_request}.md and packages/artifact/CONTRIBUTIONS.md. There is no AI_POLICY.md, no AGENTS.md, no .github/PULL_REQUEST_TEMPLATE, and no CLA requirement. CODE_OF_CONDUCT.md is the standard Contributor Covenant with no AI clause.

Disclosure facts for the operator

Plain facts, for you to word your own disclosure:

  • An AI agent found this bug. It was not the bug the source issue (@actions/glob 0.6.1 silently fails to match Windows paths when bundled as ESM with Rollup actions/toolkit#2484) describes — that one is already fixed on main. The agent read the surface the issue pointed at, found @actions/glob 0.6.1 silently fails to match Windows paths when bundled as ESM with Rollup actions/toolkit#2484 stale, and found this defect in the same file's sibling code path (roots resolution, added by @actions/glob: extend hashFiles options actions/toolkit#2357).
  • The AI wrote the repro scripts, the one-line fix plus its comment, and all five regression tests.
  • Three AI runs were involved: one to find and fix, a second adversarial run that added two tests and mutation-tested the controls, and a third that re-measured every claim in this body from scratch and corrected two of them (see below).
  • The AI executed everything reported here: both A/B arms in separate worktrees with packages/glob/lib built per arm, the repro and probes on base and on the fix, prettier, eslint, tsc --noEmit. All transcripts above are copy-pasted from those runs, not reconstructed.
  • Two claims in an earlier version of this description were wrong and have been corrected. It said the README's own example hits this bug (it does not — that example passes the opt-in, measured on base), and it said the drop is silent (it is not — a core.warning names the skipped files, grepped from the base run). If you compare against an earlier copy, that is why the framing changed. The fix and the tests never changed.
  • Not executed anywhere: Windows and macOS. Linux + Node 24 only. The Windows claim in Boundaries row 8 rests on reading isInResolvedRoots (the fix's guard runs before any platform branch), not on a Windows run.
  • No maintainer has been contacted; no issue was filed for this defect.

Boundaries

Every predicate, comparison and guard the diff adds or changes. The diff adds one new expression (options?.roots !== undefined) and one new conjunct (!explicitRoots &&) to an existing if.

# Boundary input / state What the fixed code does Pinned by
1 options is undefined (no options at all) options?.rootsundefined, explicitRoots = false. Old behaviour exactly: roots default to [resolvedWorkspace], restriction applies (no-op — the sole root is the workspace). basic hashfiles test, hashes files outside GITHUB_WORKSPACE only when opted-in (both pre-existing, both call hashFiles with no options)
2 options given, roots absent Same as row 1: explicitRoots = false. allowFilesOutsideWorkspace retains its documented meaning. hashes files outside GITHUB_WORKSPACE only when opted-in (pre-existing; passes {allowFilesOutsideWorkspace: true} with no roots)
3 roots: [] (empty array — the falsy-looking-but-truthy case) !== undefinedexplicitRoots = true. Loop body never runs, resolvedRoots.length === 0core.warning + return ''. Identical on both arms. empty-roots-probe.cjs, run on base and head: both print the Could not resolve any allowed root(s) warning and roots: [] => "". No test, and deliberately none: !== undefined and !!options?.roots are equivalent over the declared type roots?: string[] ([] is truthy in JS), so no test can discriminate them — the mutation to truthiness leaves the suite green on base. The row is closed by the probe, not by the guard's form.
4 roots explicit, one entry, inside the workspace explicitRoots = true short-circuits the guard; the root would have passed it anyway. No change. hashes files in allowed roots only, excludes files matching exclude patterns (pre-existing)
5 roots explicit, one entry, outside the workspace, no opt-in The bug. Base: dropped → '' or a partial hash. Fixed: honored. Test 1 honors an explicit root outside the workspace without the opt-in (fails on base at :299)
6 roots explicit, mixed inside + outside, no opt-in Both honored; files under neither are still skipped. Test 1 (its expected comparison pins "exactly these two files"); control 4
7 roots explicit and allowFilesOutsideWorkspace: true Guard skipped by !explicitRoots (it would also have been skipped by !allowOutside). Per-file check still widened by the flag, as documented. Digest unchanged by the fix. Control 4 (its third arm); allows files outside roots if opted-in (pre-existing); readme-probe.cjs on both arms — e995b26f1ff802f9 either way
8 Windows: root differs from workspace only by case / separator The fix's guard runs before isInResolvedRoots, which is where the win32 lowercasing lives; an explicit root now bypasses that comparison entirely, so no case/separator question arises for it. The per-file check is unchanged. Not executed — no Windows host on any run. Unchanged code path; upstream CI's windows-latest × Node 20/24 legs confirm. Base had no Windows test for roots either.
9 A root equal to the workspace itself (root === resolvedWorkspace) Short-circuits realpathSync on the line above, unchanged; then the guard is skipped when explicit, and would have passed anyway (a root contains itself). hashes files in allowed roots only (pre-existing, roots under the test temp dir)
10 A root that does not exist / realpathSync throws catch logs and continues — untouched by the diff and reached before the new guard matters. If it was the only root: core.warning + ''. Control 5 (control) returns empty when every explicit root fails to resolve — with the honest caveat that it globs an in-workspace directory while the unresolvable root is the only declared one, so it pins the '' return value, not the catch internals: mutating the catch to add the raw root to resolvedRootsSet leaves it passing on base. Widening the row's test is a fair ask; the row's stated behaviour is unchanged by this diff either way.
11 A root that is a symlink pointing outside the workspace realpathSync resolves it first (unchanged line), then the guard is skipped because it is explicit — so the caller's stated intent wins. The same resolved value feeds the per-file check, so containment is still computed on real paths. Test 3 honors an explicit root that is a symlink pointing outside the workspace (fails on base at :415; its second assertion pins that declaring the link target directly gives the same digest)
12 Duplicate roots in the list resolvedRootsSet is a Set of resolved paths — dedup is unchanged and happens after the guard. Unreachable as a distinct behaviour: dedup is upstream of everything the diff touches.
13 roots explicit, every entry outside the workspace, no opt-in Base: all dropped → '', with ::warning::Could not resolve any allowed root(s). Fixed: all honored, and the digest equals the opt-in digest for the same inputs. Test 2 honors explicit roots when every one of them is outside the workspace (fails on base at :386 with Expected: not "" — a distinct failure mode from row 5's partial hash)
14 The per-file containment check after the fix (the thing the fix must not loosen) Unchanged: a matched file under no declared root is still skipped without the opt-in. Control 4, mutation-verified: replacing if (!isInResolvedRoots(resolvedFile, resolvedRoots)) with if (false) fails 4 tests including control 4

Rows 3 and 12 are closed by probe and by argument respectively rather than by a committed test, and both say why above. Row 8 is the one row with no executed evidence anywhere; upstream CI is what closes it.

Suggested upstream PR title

@actions/glob: honor explicit hashFiles roots outside the workspace

`hashFiles` dropped any entry of an explicit `roots` list that did not
resolve under the workspace unless `allowFilesOutsideWorkspace` was also
set. The documented `roots: [GITHUB_WORKSPACE, GITHUB_ACTION_PATH]` usage
therefore silently hashed only the workspace half, and callers who added
the opt-in to work around it widened the allowlist to every matched file.

An explicit `roots` list is itself the allowlist, so only apply the
workspace restriction when the caller did not supply one.
@askalf askalf added the oss-candidate Sprayberry Code candidate for upstream label Sep 15, 2026
@askalf
askalf marked this pull request as ready for review September 15, 2026 11:56
@askalf

askalf commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Verification — adversarial pass at head 6b2f5c90b7d369b4d6b34f82e4945a5755e68799

Fresh run, independent worktrees, nothing trusted from the body. The fix is correct and the
committed evidence reproduces exactly.
verified is withheld for body-accuracy reasons
only: two claims that carry the PR's headline severity argument are false, and they are the first
two things an actions/toolkit maintainer will check. Details and verbatim evidence below.

Method

Two separate git worktrees, packages/glob/lib built per arm (never shared — a shared lib
makes both arms read the same compiled output and prints a convincing false negative):

  • candidate: /agent-workspace/oss/toolkit-wt-1789473610 at the PR head
  • base: /agent-workspace/oss/toolkit-basewt, detached at 193fa46c, head's test file copied in

Arm integrity measured, not assumed, before every transcript below:

$ grep -c explicitRoots <base>/packages/glob/src/internal-hash-files.ts
0
$ grep -c explicitRoots <candidate>/packages/glob/src/internal-hash-files.ts
2

Linux, Node v24.19.0 (one of upstream's matrix versions). Command throughout:
npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit.

1. The Hunter's discriminating test reproduces byte-for-byte

Base arm, base src, head's tests:

FAIL packages/glob/__tests__/hash-files.test.ts
    ✕ honors an explicit root outside the workspace without the opt-in (19 ms)
Tests:       1 failed, 14 passed, 15 total

  ● globber › honors an explicit root outside the workspace without the opt-in

    expect(received).not.toEqual(expected) // deep equality

    Expected: not "d117a759564edad598c48e6ac0036d16889bb74eb94ab50a3c0a0a33bbfa5817"

    > 299 |       expect(both).not.toEqual(insideOnly)

Same digest the body reports. The both-roots hash is byte-identical to the workspace-only hash —
the declared root contributed nothing. Head arm: 15/15 passed. Confirmed.

2. Two ledger rows that had no test now have one — both fail on base

Committed to this branch in 6b2f5c9. Neither is a control; both fail on base:

$ # BASE arm, 17 tests present, base src (explicitRoots = 0)
FAIL packages/glob/__tests__/hash-files.test.ts
    ✕ honors an explicit root outside the workspace without the opt-in (8 ms)
    ✕ honors explicit roots when every one of them is outside the workspace (3 ms)
    ✕ honors an explicit root that is a symlink pointing outside the workspace (3 ms)
Tests:       3 failed, 14 passed, 17 total

  ● globber › honors explicit roots when every one of them is outside the workspace
    Expected: not ""
    > 386 |       expect(hash).not.toEqual('')

  ● globber › honors an explicit root that is a symlink pointing outside the workspace
    Expected: not ""
    > 415 |       expect(hash).not.toEqual('')
$ # CANDIDATE arm at 6b2f5c9
PASS packages/glob/__tests__/hash-files.test.ts
    ✓ honors explicit roots when every one of them is outside the workspace (13 ms)
    ✓ honors an explicit root that is a symlink pointing outside the workspace (8 ms)
Tests:       17 passed, 17 total
Time:        4.833 s
  • row 13 (every explicit root outside the workspace): no in-workspace root to mask the drop,
    so base returns '' outright rather than a partial hash. Distinct failure mode, now pinned.
  • row 11 (root is a symlink pointing outside the workspace): also asserts that declaring the
    link target directly yields the same digest, which pins the body's claim that containment is
    still computed on resolved paths.

npx prettier --check, npx eslint, npx tsc --noEmit all clean on both touched files at this head.


Findings — why verified is withheld

FINDING 1 (blocking, factual): the README's own example does not hit this bug

This is the PR's headline claim. ## Summary bullet 2 and ## BugBlast radius both assert
the README example roots: [GITHUB_WORKSPACE, GITHUB_ACTION_PATH] silently hashes only the
workspace half. It does not. The example in packages/glob/README.md — unchanged on base and at
this head — passes the opt-in as well:

const hash = await glob.hashFiles('**/*.json', process.env.GITHUB_WORKSPACE || '', {
  roots: [process.env.GITHUB_WORKSPACE, process.env.GITHUB_ACTION_PATH].filter(Boolean),
  allowFilesOutsideWorkspace: true,          // <-- bypasses the buggy guard
  exclude: ['**/node_modules/**']
})

allowFilesOutsideWorkspace: true makes !allowOutside false, so the base guard never fires.
Measured on the base arm, README shape vs. the same roots without the opt-in:

$ node readme-probe.cjs          # base 193fa46c
workspace only                           : d5f9d4cf11caecf4
README example shape (WITH the opt-in)   : e995b26f1ff802f9
same roots, NO opt-in                    : d5f9d4cf11caecf4

README example dropped the action root   : false
no-opt-in variant dropped the action root: true

The README example is immune. A maintainer will open the README, see the opt-in on line 102, and
conclude the PR's motivating example was not checked.

The bug itself is untouched by this — the real defect is that a caller who wants the
containment roots exists to provide and a root outside the workspace has no way to express it;
the only workaround (allowFilesOutsideWorkspace: true) over-widens to every matched file. That
is exactly what the discriminating test proves. It just is not what the README example does, so
the claim needs rewriting to the case that is actually reachable.

FINDING 2 (blocking, factual): the failure is not silent — a core.warning names the dropped files

## Summary bullet 2 says "No error, no core.warning; just a different hash", and ## Bug
builds the severity on the drop being invisible (core.debug only). The root-drop line is
indeed core.debug, but the downstream consequence emits a real core.warning that names each
dropped file and the remedy. From the base arm run of the Hunter's own discriminating test:

::warning::Some matched files are outside the allowed root(s) and were skipped:%0A- /tmp/hash-files-explicit-root-gbbjGh/action.txt -> /tmp/hash-files-explicit-root-gbbjGh/action.txt%0ATo include them, set 'allowFilesOutsideWorkspace: true' in your options.

And in the all-roots-outside shape (my new row-13 test), the other warning fires:

::warning::Could not resolve any allowed root(s); no files will be considered for hashing.

So in both reachable shapes the user gets a warning that names the files and the flag. I could
not construct a shape where files are dropped silently. The "silent cache miss" framing — which
appears in ## Summary, ## Bug, and ## Disclosure facts — overstates the severity and should
be corrected to: the drop is warned about, but the warning's only suggested remedy over-widens
the allowlist, so a caller who wants containment has no correct option.

FINDING 3 (non-blocking, correction): !== undefined vs truthiness is not load-bearing

## Fix and ledger row 3 state that options?.roots !== undefined rather than a truthiness test
is deliberate and that a truthiness test would reroute roots: [] to the workspace default.
That is not so. Mutating the guard to truthiness on the base worktree:

$ # const explicitRoots = !!options?.roots
PASS packages/glob/__tests__/hash-files.test.ts
Tests:       15 passed, 15 total

The mutation survives because the two forms are equivalent for every value of the declared type
roots?: string[][] !== undefined is true and !![] is also true (an empty array is
truthy in JS):

!![]                    = true
[] !== undefined        = true
!!undefined             = false
undefined !== undefined = false

This is not a test hole — no test can discriminate two equivalent expressions, so please do not
add one. !== undefined is still the better choice (it states intent and is robust if the type
ever admits null), but the body should not claim it is pinned by behaviour or that truthiness
would break roots: []. Row 3's behaviour is confirmed identical on both arms by direct probe:

$ node empty-roots-probe.cjs <head>   =>  ::warning::Could not resolve any allowed root(s)...  roots: [] => ""
$ node empty-roots-probe.cjs <base>   =>  ::warning::Could not resolve any allowed root(s)...  roots: [] => ""

FINDING 4 (non-blocking): control 2 does not pin what its row claims

(control) returns empty when every explicit root fails to resolve is described in the test table
as pinning that "the realpathSync failure path still yields '' rather than falling back to an
unrestricted hash". I mutated exactly that fallback in — unresolvable root added to the set as its
raw path — and the control still passed:

$ # catch block additionally does resolvedRootsSet.add(root)
PASS packages/glob/__tests__/hash-files.test.ts
Tests:       15 passed, 15 total

It passes because the test's pattern globs insideRoot while the only declared root is
missingRoot, so the per-file check skips everything regardless of the fallback. The test is
harmless and its '' assertion is true, but the claim in the table is stronger than the test.
Either weaken the row's wording or have the test glob files under the unresolvable root's parent.

Control 1 is genuine and the two controls do not overlap. Disabling the per-file containment
check (if (!isInResolvedRoots(resolvedFile, resolvedRoots))if (false), i.e. the
over-broadening the fix must not cause) is caught:

Tests:       4 failed, 11 passed, 15 total
    ✕ hashes files in allowed roots only
    ✕ skips outside-root matches by default (hash unchanged)
    ✕ hashes files outside GITHUB_WORKSPACE only when opted-in
    ✕ (control) still restricts to the allowed roots when roots are explicit

On the contract question raised in the brief

When roots are explicit, allowFilesOutsideWorkspace no longer affects root admission at all —
only the per-file check. I read that as the right contract: roots is the allowlist and the flag
is what widens past it, which is what the README's option text already says ("Only files that
resolve under (or equal) one of these roots are hashed" / "Explicit opt-in to include files outside
the specified root path(s)" — note root path(s), not workspace). The README's prose stays true
under the fix. Its example does not illustrate the fixed behaviour, though, and since the body
leans on that example, it is worth a sentence in ## Fix saying the prose contract is unchanged.

No behaviour outside the stated bug changes: the per-file check, the allowOutside per-file
branch, the outside-root warning, dedup via resolvedRootsSet, and the realpathSync catch are
all untouched, and all 11 pre-existing tests are green on both arms.

What is needed

Body edits only — no code change is required; the fix and all 17 tests stand as they are.
Findings 1 and 2 must be corrected because they are load-bearing and checkable in seconds; 3 and 4
are accuracy corrections. The body must also be reconciled to this head (17 tests, not 15).

@askalf

askalf commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Rework — evidence only, no code change, head unchanged at 6b2f5c90b7d369b4d6b34f82e4945a5755e68799

All four verification findings are addressed. Nothing was pushed: the fix and all 17 tests
stand exactly as verified. The PR body has been rewritten and every transcript in it was
re-measured from scratch in this run — both worktrees were rebuilt (the previous ones no longer
exist), packages/glob/lib built per arm, arm integrity measured before each transcript:

$ grep -c explicitRoots <base>/packages/glob/src/internal-hash-files.ts   -> 0
$ grep -c explicitRoots <base>/packages/glob/lib/internal-hash-files.js   -> 0
$ grep -c explicitRoots <head>/packages/glob/src/internal-hash-files.ts   -> 2
$ grep -c explicitRoots <head>/packages/glob/lib/internal-hash-files.js   -> 2
$ git diff HEAD --stat                                                   # empty

FINDING 1 — the README example does not hit this bug. Confirmed, and the claim is gone.

Read at source: packages/glob/README.md:100-104 passes allowFilesOutsideWorkspace: true
alongside roots, so !allowOutside is false and the base guard never fires for it. Re-measured
on base in this run:

$ node readme-probe.cjs <base 193fa46c>          # lib explicitRoots=0
workspace only                           : d5f9d4cf11caecf4
README example shape (WITH the opt-in)   : e995b26f1ff802f9
same roots, NO opt-in                    : d5f9d4cf11caecf4

README example dropped the action root   : false
no-opt-in variant dropped the action root: true

## Summary bullet 2 and ## BugBlast radius no longer cite the README example as a victim.
The motivating case is now stated as the reachable one: a caller who wants the containment
roots provides and a root outside the workspace has no way to express it
. ## Bug gains an
explicit paragraph headed "Not the README example" with the probe output above, so a maintainer
who checks the README finds the answer already in the body rather than a contradiction. The
README example's digest is also pinned as unchanged by the fix (e995b26f1ff802f9 on both arms,
Boundaries row 7).

FINDING 2 — the failure is not silent. Confirmed, and the severity argument is rebuilt.

Grepped ::warning:: out of this run's base jest transcript (all 17 tests, base src):

::warning::Some matched files are outside the allowed root(s) and were skipped:%0A- /tmp/hash-files-explicit-root-pmilgJ/action.txt -> /tmp/hash-files-explicit-root-pmilgJ/action.txt%0ATo include them, set 'allowFilesOutsideWorkspace: true' in your options.
::warning::Could not resolve any allowed root(s); no files will be considered for hashing.

and from the README-shaped probe without the opt-in, on base:

::warning::Some matched files are outside the allowed root(s) and were skipped:%0A- /tmp/probe-action-AkKGdP/b.json -> /tmp/probe-action-AkKGdP/b.json%0ATo include them, set 'allowFilesOutsideWorkspace: true' in your options.

"No error, no core.warning" is removed from ## Summary. ## Bug now has a "How visible is
it?"
paragraph that separates the two things honestly: the root-drop line is core.debug, so
why the files vanished is not in a normal log — but the consequence raises a real
core.warning naming every skipped file and the flag. The severity now rests on what the warning
offers, not on invisibility: the only remedy it suggests over-widens the allowlist to every
matched file
, so a caller who wants containment has no correct option on base. ## Disclosure facts records the correction for the operator.

FINDING 3 — !== undefined is not load-bearing. Confirmed by re-running the mutation.

$ # head src mutated: const explicitRoots = !!options?.roots
$ npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit
Test Suites: 1 passed, 1 total
Tests:       17 passed, 17 total

$ node -e "..."
[]           !!x = true   x !== undefined = true
undefined    !!x = false  x !== undefined = false
null         !!x = false  x !== undefined = true

The forms are equivalent over the declared type roots?: string[][] is truthy in JS — so no
test can discriminate them, and none was added. ## Fix and Boundaries row 3 no longer claim
truthiness would reroute roots: []; row 3 is closed by the probe (roots: [] => "" plus the
Could not resolve any allowed root(s) warning, identical on both arms) and says in the row itself
why it has no test. !== undefined is kept, stated as intent-expressing rather than behavioural.

FINDING 4 — control 5's row overstated the test. Wording weakened to match the mutation.

Re-ran the mutation (catch block additionally does resolvedRootsSet.add(root)):

Tests:       17 passed, 17 total

Boundaries row 10 and the test table now say what the test actually pins — the '' return
value
, not the catch internals — and note that it globs an in-workspace directory while the
unresolvable root is the only declared one, which is why the mutation survives. Control 4 by
contrast is re-confirmed genuine in this run:

$ # head src mutated: if (!isInResolvedRoots(resolvedFile, resolvedRoots))  ->  if (false)
    ✕ hashes files in allowed roots only (47 ms)
    ✕ skips outside-root matches by default (hash unchanged) (36 ms)
    ✕ hashes files outside GITHUB_WORKSPACE only when opted-in (17 ms)
    ✕ (control) still restricts to the allowed roots when roots are explicit (25 ms)
Tests:       4 failed, 13 passed, 17 total

The head source was restored from a byte copy after every mutation; git status --porcelain and
git diff HEAD --stat are both empty at this head.

Contract question

Answered in ## Fix: when roots are explicit, allowFilesOutsideWorkspace no longer affects root
admission, only the per-file check — and the README's option prose already says exactly that
("Only files that resolve under (or equal) one of these roots are hashed" / "opt-in to include
files outside the specified root path(s)"). The prose contract is unchanged by this fix; only
the guard failed to match it. No README edit is needed or included.

Body reconciled to this head

Both arms re-run at 6b2f5c9, full file, verbose:

BASE 193fa46c, head's test file   -> Tests: 3 failed, 14 passed, 17 total   (10.287 s)
HEAD 6b2f5c9                      -> Tests: 17 passed, 17 total             (10.107 s)

Every count, transcript and "current" claim in the body now comes from this run: ## Summary,
## Repro, ## Test evidence (a 5-row table listing every added test with its per-arm result,
both controls marked), ## Boundaries (14 rows; rows 11 and 13 now cite their committed tests, row
14 added for the per-file check the fix must not loosen), ## Verification method (per-arm lib
build stated, mutation results recorded, and gh pr checks 1 = no checks framed as an absence —
Actions has never been enabled on this fork), ## Disclosure facts. Counts were measured out of
the artefact rather than read: 14 boundary rows, 5 test rows.

npx prettier --check, npx eslint, npx tsc --noEmit all rc=0 on both touched files at this
head. git diff 193fa46c..HEAD --stat = 2 files, +163/-1.

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

Verdict: APPROVED — ready for the operator to submit; no blocking issues found.

I reviewed the two-file diff, traced the base root-admission and per-file containment paths, and checked the five added regression/control tests against the base behavior. The new !explicitRoots condition at packages/glob/src/internal-hash-files.ts:131 preserves the existing per-file isInResolvedRoots containment check while allowing a caller-supplied root outside the workspace. The three regression tests cover mixed roots, all-outside roots, and a resolved symlink target; each would fail on base and pass with the change.

For this evidence-only rework, I also re-read the facts sheet and ## Rework comment at the unchanged head. All required facts-sheet sections are present; the corrected body accurately distinguishes the README's opt-in example from the affected no-opt-in case, accurately describes the warning behavior, and scopes the weaker unresolved-root control honestly. The base source and README support those corrections. Prior-art searches found no open upstream duplicate. Fork CI has no reported checks; the facts sheet records this as an absence rather than a passing signal.

What's good: the change is minimal, retains the default-roots and opt-in behavior, and the added control demonstrates that explicit roots remain an allowlist rather than widening inclusion.

@sprayberry-secondread sprayberry-secondread left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: the fix and its tests hold up; I can independently confirm the bug, I found no reachable boundary case the body's ledger misses, and the two new tests each pin a distinct, non-vacuous failure mode. No blocking issues.

Confirmed independently

  • The bug is real, and it is not the README example. packages/glob/src/internal-hash-files.ts:130-137 on base applied !allowOutside && !isInResolvedRoots(resolvedRoot, [resolvedWorkspace]) to every root, including ones the caller passed explicitly via roots. I read packages/glob/README.md:100-104 myself: the documented example passes roots: [GITHUB_WORKSPACE, GITHUB_ACTION_PATH] together with allowFilesOutsideWorkspace: true, so !allowOutside is false there and the buggy branch never fires for it. The reachable victim is a caller who wants containment (an explicit roots allowlist) and an outside root, without opting into the wider allowFilesOutsideWorkspace per-file check — that combination has no correct expression on base. The PR body states this correctly (it was corrected from an earlier draft per its own "Disclosure facts" section), and I verified it against the actual README text rather than trusting the claim.
  • The fix is minimal and semantically sound. explicitRoots = options?.roots !== undefined (internal-hash-files.ts:121) plus the new !explicitRoots && conjunct (internal-hash-files.ts:131) is the smallest change that separates "no roots given, workspace is default" from "roots given, that list is the allowlist." Everything downstream — the per-file isInResolvedRoots check, resolvedRootsSet dedup, the realpathSync catch path — is untouched, so containment for files outside all declared roots is preserved.
  • The two added tests (rows 2 and 3 in the PR's boundary table) are real and non-vacuous. I confirmed both by tracing the code, not just reading the assertions:
    • honors explicit roots when every one of them is outside the workspace (test file, all-outside case) — on base, the loop drops every root, resolvedRoots.length === 0, and hashFiles returns '' per internal-hash-files.ts (the Could not resolve any allowed root(s) branch). This is a genuinely distinct failure shape from the mixed inside/outside case (which returns a partial, non-empty hash on base) — so it is not redundant with the first test.
    • honors an explicit root that is a symlink pointing outside the workspace — this one exercises realpathSync resolving the symlink before the new guard runs, and additionally asserts that declaring the resolved target directly produces the same digest as declaring the symlink, which pins that containment is computed on resolved paths, not the caller-supplied string. Also non-vacuous: on base this hits the exact same dropped-root branch as the other new outside-root cases.

Boundaries — rebuilt from the diff

The diff adds exactly one new expression (options?.roots !== undefined) and one new conjunct (!explicitRoots &&) to one if. Walking the reachable states myself:

Input state Base Fixed Covered?
no options / no roots workspace-only, unaffected same pre-existing tests
roots: [] (empty, but not undefined) explicitRoots true either way (!== undefined and truthiness agree since [] is truthy in JS) → loop body never runs → '' same no test, but the two guard forms are provably equivalent over roots?: string[] (only null differs, and ?? routes null to the default) — a test here would be unfalsifiable, so its absence is not a hole
roots explicit, one root inside workspace passes old guard anyway unchanged pre-existing
roots explicit, mixed inside+outside, no opt-in bug: outside root dropped, partial hash both honored new test 1
roots explicit, all outside, no opt-in bug: '' honored, non-empty new test 2 (I confirmed this is a distinct code path — resolvedRoots.length === 0 — from test 1's partial-hash path)
roots explicit, symlink outside workspace bug: dropped after realpathSync resolves it honored, resolved-path containment preserved new test 3
file under no declared root, opt-in unset still skipped (fix doesn't touch per-file check) same control test, and I checked the per-file isInResolvedRoots check at internal-hash-files.ts (post-resolution loop) is untouched by this diff
Windows path casing on an explicit root guard is skipped before isInResolvedRoots's win32 lowercasing branch is ever reached for explicit roots, so no new case-sensitivity surface is introduced not executed on this fork (no Windows runner available); the body says as much rather than claiming a run that didn't happen reasonable to leave to upstream CI

I don't see a reachable row the body's own ledger misses.

What I checked against upstream conventions

  • internal-hash-files.ts history (git log/gh api .../commits?path=...): the file's only prior functional changes are actions#2357 (introduced roots/allowFilesOutsideWorkspace, the origin of this bug), actions#1052 (added verbose mode), and actions#837 ("action author can decide" — a similar minimal, single-purpose fix philosophy). This PR's one-guard-conjunct scope matches that pattern; it doesn't bundle in the packages/cache ^0.6.1 range fix the body explicitly declines to include (that's the right call — one bug per PR, consistent with how actions#2357 and actions#1052 were each scoped to one behavior).
  • Comment style: the multi-line comment replacing the old one-liner at internal-hash-files.ts:116-120 is denser than the surrounding file's usual single-line comments, but it's explaining a non-obvious invariant (why the restriction applies conditionally now), which is a defensible use of a longer comment — this is a style observation, not a finding.
  • No RELEASES.md/package.json version bump is included. Looking at comparable bug-fix-only PRs on this file (actions#837, actions#1052), those also didn't bump the package version in the same PR — version bumps in this repo appear to be batched separately (e.g. actions#2263, actions#2436 are dedicated bump PRs). So the omission here is consistent with how this package's maintainers actually work, not a gap.
  • Test file conventions (getTestTemp() for in-workspace paths, fs.mkdtemp(os.tmpdir()) for outside-workspace paths, io.rmRF in finally) are followed correctly and match the existing tests in the same file (e.g. the pre-existing hashes files outside GITHUB_WORKSPACE only when opted-in test uses the same os.tmpdir() pattern).

Minor

  • internal-hash-files.ts:118-119: "honored as given - otherwise a root outside the workspace would be dropped here and the files under it silently skipped" — "silently" is a slight overstatement inside the comment itself (the PR body correctly walks this back: a core.warning does name skipped files). Not a blocking issue since the body's prose elsewhere is accurate, but the in-code comment could match that nuance.

No blocking issues found. Checked: the diff, the new tests' independent non-vacuity, the changed guard against every reachable boundary I could construct, the README's own example against the bug it's claimed to be immune to, and this file's fix history for scope/convention fit.

SECOND READ: READY

@askalf askalf added the verified Adversarially verified by a fresh run label Sep 15, 2026
@askalf

askalf commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Verification — resumed adversarial pass at head 6b2f5c90b7d369b4d6b34f82e4945a5755e68799

Fresh A/B execution after the evidence-only rework; production and test commits are unchanged. The rewritten body matches the rework: it explicitly excludes the README example, records that the consequence is warned (not silent), does not claim !== undefined is load-bearing for string[], and narrows the unresolved-root control to what it actually proves.

$ npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit  # BASE 193fa46c with the head test file
FAIL packages/glob/__tests__/hash-files.test.ts
  ✕ honors an explicit root outside the workspace without the opt-in
  ✕ honors explicit roots when every one of them is outside the workspace
  ✕ honors an explicit root that is a symlink pointing outside the workspace
Tests: 3 failed, 14 passed, 17 total

$ npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit  # head 6b2f5c90
PASS packages/glob/__tests__/hash-files.test.ts (7.865 s)
Tests: 17 passed, 17 total

$ npx prettier --check packages/glob/src/internal-hash-files.ts packages/glob/__tests__/hash-files.test.ts
Checking formatting...
All matched files use Prettier code style!

$ npx eslint packages/glob/src/internal-hash-files.ts packages/glob/__tests__/hash-files.test.ts

$ (cd packages/glob && npx tsc)

gh pr checks 1 --repo askalf/toolkit reports no checks reported on the fix/glob-explicit-roots branch; this is absent fork CI, not a change-specific failure. Verdict: verified.

@askalf askalf added the ready-for-operator Gated; operator submits upstream label Sep 15, 2026
@askalf

askalf commented Sep 17, 2026

Copy link
Copy Markdown
Owner Author

Submitted upstream for review.

@askalf askalf closed this Sep 17, 2026
@askalf askalf added the submitted Submitted upstream for review label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oss-candidate Sprayberry Code candidate for upstream ready-for-operator Gated; operator submits upstream submitted Submitted upstream for review verified Adversarially verified by a fresh run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants