Skip to content

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

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

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

Conversation

@askalf

@askalf askalf commented Sep 15, 2026

Copy link
Copy Markdown

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 main.
  • 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 #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.
$ node repro.cjs <base checkout>      # 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 <fixed checkout>   # this branch
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
$ npx jest packages/glob/__tests__/hash-files.test.ts --runInBand --forceExit   # BASE, this PR's 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   # this branch
PASS packages/glob/__tests__/hash-files.test.ts
Tests:       17 passed, 17 total

Decisions

   // 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: with 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. With roots given, the caller has stated the allowlist, so it is honored.

Everything downstream is untouched: the per-file isInResolvedRoots 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.

The documented contract is unchanged: 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 needed 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 since [] is truthy in JS and only null distinguishes the two forms, which ?? [resolvedWorkspace] sends to the default either way.

Alternatives rejected: emitting a warning while keeping the behaviour doesn't help, since a warning already exists and the problem is that the documented GITHUB_ACTION_PATH usage remains impossible without over-widening; restricting against the union of the workspace and all supplied roots is self-referential and reduces to this fix with more code; documenting the flag as required alongside outside-workspace roots is exactly the compromise this fix removes, since the opt-in is all-or-nothing per file and makes containment unavailable to the callers who most need it.

Also noted, deliberately not in this PR: packages/cache/package.json still declares "@actions/glob": "^0.6.1", a range that predates this fix — a separate dependency-bump change, one bug per PR.

Not run: the full monorepo test suite (npm test at root) — scoped to packages/glob per the repository's own guidance for package-level changes; npm run format and npm run lint were run and are clean.

AI assistance: this bug was found and the fix and tests were drafted with AI tooling in my workflow; the tests and checks above were executed as pasted. I'm responsible for the change and will handle review feedback.

`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 requested a review from a team as a code owner September 15, 2026 16:58
@askalf askalf changed the title test @actions/glob: honor explicit hashFiles roots outside the workspace Sep 15, 2026
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.

1 participant