Skip to content

Commit 03ef782

Browse files
authored
Fix repo-memory glob matching for nested memory files (#44215)
1 parent 9c0ce8f commit 03ef782

10 files changed

Lines changed: 385 additions & 42 deletions

File tree

actions/setup/js/glob_pattern_helpers.cjs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ function escapeRegexChars(pattern) {
2020
* @param {Object} [options] - Options for pattern conversion
2121
* @param {boolean} [options.pathMode=true] - If true, * matches non-slash chars; if false, * matches any char
2222
* @param {boolean} [options.caseSensitive=true] - Whether matching should be case-sensitive
23+
* @param {boolean} [options.matchSubfolderRoot=false] - If true and the pattern contains no "/",
24+
* the pattern is required to be prefixed by exactly one path segment (i.e. matches only at the
25+
* root of a single memory subfolder: "subfolder/file.json"). Files at the artifact root (depth 0)
26+
* and files deeper than one level (depth 2+) are not matched.
2327
* @returns {RegExp} - Regular expression that matches the pattern
2428
*
2529
* Supports:
@@ -37,9 +41,15 @@ function escapeRegexChars(pattern) {
3741
* const regex = globPatternToRegex("metrics/**");
3842
* regex.test("metrics/data.json"); // true
3943
* regex.test("metrics/daily/data.json"); // true
44+
*
45+
* @example
46+
* const regex = globPatternToRegex("*.json", { matchSubfolderRoot: true });
47+
* regex.test("discussion-task-miner/processed-discussions.json"); // true (depth 1)
48+
* regex.test("file.json"); // false (depth 0 – not matched)
49+
* regex.test("discussion-task-miner/archive/old.json"); // false (depth 2 – not matched)
4050
*/
4151
function globPatternToRegex(pattern, options) {
42-
const { pathMode = true, caseSensitive = true } = options || {};
52+
const { pathMode = true, caseSensitive = true, matchSubfolderRoot = false } = options || {};
4353

4454
let regexPattern = escapeRegexChars(pattern);
4555

@@ -54,6 +64,13 @@ function globPatternToRegex(pattern, options) {
5464
regexPattern = regexPattern.replace(/\*/g, ".*");
5565
}
5666

67+
// matchSubfolderRoot: slashless patterns must match exactly at the root of one subfolder
68+
// (depth 1 only – neither depth 0 nor depth 2+).
69+
// Patterns that already contain a "/" are matched against the full relative path unchanged.
70+
if (matchSubfolderRoot && !pattern.includes("/")) {
71+
regexPattern = `[^/]+/${regexPattern}`;
72+
}
73+
5774
return new RegExp(`^${regexPattern}$`, caseSensitive ? "" : "i");
5875
}
5976

actions/setup/js/glob_pattern_helpers.test.cjs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,57 @@ describe("glob_pattern_helpers.cjs", () => {
359359
});
360360
});
361361

362+
describe("matchSubfolderRoot option", () => {
363+
it("slashless pattern matches only at subfolder root (depth 1)", () => {
364+
const regex = globPatternToRegex("*.json", { matchSubfolderRoot: true });
365+
366+
// depth 1 – the intended match
367+
expect(regex.test("discussion-task-miner/processed-discussions.json")).toBe(true);
368+
expect(regex.test("subfolder/file.json")).toBe(true);
369+
370+
// depth 0 – must NOT match
371+
expect(regex.test("file.json")).toBe(false);
372+
373+
// depth 2+ – must NOT match
374+
expect(regex.test("discussion-task-miner/archive/old.json")).toBe(false);
375+
expect(regex.test("a/b/c/file.json")).toBe(false);
376+
});
377+
378+
it("slashless pattern with *.md matches at subfolder root only", () => {
379+
const regex = globPatternToRegex("*.md", { matchSubfolderRoot: true });
380+
381+
expect(regex.test("discussion-task-miner/latest-run.md")).toBe(true);
382+
expect(regex.test("latest-run.md")).toBe(false);
383+
expect(regex.test("discussion-task-miner/docs/latest-run.md")).toBe(false);
384+
});
385+
386+
it("pattern with slash is unaffected by matchSubfolderRoot", () => {
387+
const regex = globPatternToRegex("metrics/**", { matchSubfolderRoot: true });
388+
389+
expect(regex.test("metrics/file.json")).toBe(true);
390+
expect(regex.test("metrics/daily/file.json")).toBe(true);
391+
expect(regex.test("other/file.json")).toBe(false);
392+
});
393+
394+
it("default extension globs match discussion-task-miner layout", () => {
395+
const patternStrs = ["*.json", "*.jsonl", "*.csv", "*.md"];
396+
const patterns = patternStrs.map(p => globPatternToRegex(p, { matchSubfolderRoot: true }));
397+
398+
// discussion-task-miner state files (depth 1) should all match
399+
expect(patterns.some(p => p.test("discussion-task-miner/processed-discussions.json"))).toBe(true);
400+
expect(patterns.some(p => p.test("discussion-task-miner/latest-run.md"))).toBe(true);
401+
402+
// root-level files (depth 0) should NOT match
403+
expect(patterns.some(p => p.test("processed-discussions.json"))).toBe(false);
404+
405+
// too deep (depth 2+) should NOT match
406+
expect(patterns.some(p => p.test("discussion-task-miner/archive/old.json"))).toBe(false);
407+
408+
// non-matching extension should not match
409+
expect(patterns.some(p => p.test("discussion-task-miner/script.js"))).toBe(false);
410+
});
411+
});
412+
362413
describe("simpleGlobToRegex", () => {
363414
it("should match exact patterns without wildcards", () => {
364415
const regex = simpleGlobToRegex("copilot");

actions/setup/js/push_repo_memory.cjs

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,17 @@ async function main() {
137137
return;
138138
}
139139

140+
// Validate FILE_GLOB_FILTER patterns: absolute paths (starting with "/") are not supported.
141+
if (fileGlobFilter) {
142+
const allPatternStrs = fileGlobFilter.trim().split(/\s+/).filter(Boolean);
143+
for (const pat of allPatternStrs) {
144+
if (pat.startsWith("/")) {
145+
core.setFailed(`FILE_GLOB_FILTER contains an unsupported pattern: "${pat}". Patterns must not start with "/" (absolute paths are not allowed).`);
146+
return;
147+
}
148+
}
149+
}
150+
140151
// Source directory with memory files (artifact location)
141152
// The artifactDir IS the memory directory (no nested structure needed)
142153
const sourceMemoryPath = artifactDir;
@@ -274,12 +285,24 @@ async function main() {
274285

275286
// Recursively scan and collect files from artifact directory
276287
let filesToCopy = [];
277-
278-
// Log the file glob filter configuration
288+
/** @type {Array<{path: string, reason: string}>} */
289+
let filteredOutFiles = [];
290+
291+
// Compile glob patterns once, outside the scan loop
292+
/** @type {string[]} */
293+
let patternStrs = [];
294+
/** @type {RegExp[]} */
295+
let compiledPatterns = [];
279296
if (fileGlobFilter) {
280-
core.info(`File glob filter enabled: ${fileGlobFilter}`);
281-
const patternCount = fileGlobFilter.trim().split(/\s+/).filter(Boolean).length;
282-
core.info(`Number of patterns: ${patternCount}`);
297+
patternStrs = fileGlobFilter.trim().split(/\s+/).filter(Boolean);
298+
// Slashless patterns (e.g. "*.json") are matched at the root of a single memory subfolder
299+
// (depth 1 only). Patterns that already contain "/" are matched against the full relative
300+
// path unchanged.
301+
compiledPatterns = patternStrs.map(pattern => globPatternToRegex(pattern, { matchSubfolderRoot: !pattern.includes("/") }));
302+
core.info(`File glob filter enabled with ${patternStrs.length} pattern(s):`);
303+
patternStrs.forEach((pat, idx) => {
304+
core.info(` [${idx + 1}] "${pat}" -> regex: ${compiledPatterns[idx].source}`);
305+
});
283306
} else {
284307
core.info("No file glob filter - all files will be accepted");
285308
}
@@ -297,41 +320,24 @@ async function main() {
297320
const relativeFilePath = relativePath ? path.join(relativePath, entry.name) : entry.name;
298321

299322
if (entry.isDirectory()) {
323+
core.info(` [dir] ${relativeFilePath}/`);
300324
// Recursively scan subdirectory
301325
scanDirectory(fullPath, relativeFilePath);
302326
} else if (entry.isFile()) {
303327
const stats = fs.statSync(fullPath);
328+
const normalizedRelPath = relativeFilePath.replace(/\\/g, "/");
304329

305330
// Validate file name patterns if filter is set
306-
if (fileGlobFilter) {
307-
const patternStrs = fileGlobFilter.trim().split(/\s+/).filter(Boolean);
308-
const patterns = patternStrs.map(pattern => globPatternToRegex(pattern));
309-
310-
// Test patterns against the relative file path within the memory directory
311-
// Patterns are specified relative to the memory artifact directory, not the branch path
312-
const normalizedRelPath = relativeFilePath.replace(/\\/g, "/");
313-
314-
// Enhanced logging: Show what we're testing (use info for first file to aid debugging)
315-
core.debug(`Testing file: ${normalizedRelPath}`);
316-
core.debug(`File glob filter: ${fileGlobFilter}`);
317-
core.debug(`Number of patterns: ${patterns.length}`);
318-
319-
const matchResults = patterns.map((pattern, idx) => {
331+
if (compiledPatterns.length > 0) {
332+
const matchResults = compiledPatterns.map((pattern, idx) => {
320333
const matches = pattern.test(normalizedRelPath);
321-
core.debug(` Pattern ${idx + 1}: "${patternStrs[idx]}" -> ${pattern.source} -> ${matches ? "✓ MATCH" : "✗ NO MATCH"}`);
334+
core.info(` [test] ${normalizedRelPath} pattern[${idx + 1}] "${patternStrs[idx]}" -> ${matches ? "✓ match" : "✗ no match"}`);
322335
return matches;
323336
});
324337

325338
if (!matchResults.some(m => m)) {
326-
// Enhanced warning with more context about the filtering issue
327-
core.warning(`Skipping file that does not match allowed patterns: ${normalizedRelPath}`);
328-
core.info(` File path being tested (relative to artifact): ${normalizedRelPath}`);
329-
core.info(` Configured patterns: ${fileGlobFilter}`);
330-
patterns.forEach((pattern, idx) => {
331-
core.info(` Pattern: "${patternStrs[idx]}" -> Regex: ${pattern.source} -> ${matchResults[idx] ? "✅ MATCH" : "❌ NO MATCH"}`);
332-
});
333-
core.info(` Note: Patterns are matched against the full relative file path from the artifact directory.`);
334-
core.info(` If patterns include directory prefixes (like 'branch-name/'), ensure files are organized that way in the artifact.`);
339+
core.info(` [skip] ${normalizedRelPath} (${stats.size} bytes) — no pattern matched`);
340+
filteredOutFiles.push({ path: normalizedRelPath, reason: "no pattern matched" });
335341
// Skip this file instead of failing - it may be from a previous run with different patterns
336342
return;
337343
}
@@ -344,6 +350,7 @@ async function main() {
344350
throw new Error("File size validation failed");
345351
}
346352

353+
core.info(` [keep] ${normalizedRelPath} (${stats.size} bytes)`);
347354
filesToCopy.push({
348355
relativePath: relativeFilePath,
349356
source: fullPath,
@@ -354,13 +361,18 @@ async function main() {
354361
}
355362

356363
try {
364+
core.info(`Scanning artifact directory: ${sourceMemoryPath}`);
357365
scanDirectory(sourceMemoryPath);
358-
core.info(`Scan complete: Found ${filesToCopy.length} file(s) to copy`);
366+
core.info(`Scan complete: ${filesToCopy.length} file(s) accepted, ${filteredOutFiles.length} file(s) filtered out`);
367+
if (filteredOutFiles.length > 0) {
368+
core.info(`Filtered-out files (${filteredOutFiles.length}):`);
369+
filteredOutFiles.forEach(f => core.info(` - ${f.path} (${f.reason})`));
370+
}
359371
if (filesToCopy.length > 0 && filesToCopy.length <= 10) {
360-
core.info("Files found:");
372+
core.info("Accepted files:");
361373
filesToCopy.forEach(f => core.info(` - ${f.relativePath} (${f.size} bytes)`));
362374
} else if (filesToCopy.length > 10) {
363-
core.info(`First 10 files:`);
375+
core.info(`Accepted files (first 10 of ${filesToCopy.length}):`);
364376
filesToCopy.slice(0, 10).forEach(f => core.info(` - ${f.relativePath} (${f.size} bytes)`));
365377
core.info(` ... and ${filesToCopy.length - 10} more`);
366378
}

actions/setup/js/push_repo_memory.test.cjs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,6 +1446,114 @@ describe("push_repo_memory.cjs - shell injection security tests", () => {
14461446
});
14471447
});
14481448

1449+
describe("push_repo_memory.cjs - FILE_GLOB_FILTER validation", () => {
1450+
let mockCore, mockContext, mockFs, mockExecGitSync;
1451+
1452+
beforeEach(() => {
1453+
mockCore = {
1454+
info: vi.fn(),
1455+
warning: vi.fn(),
1456+
error: vi.fn(),
1457+
debug: vi.fn(),
1458+
setFailed: vi.fn(),
1459+
setOutput: vi.fn(),
1460+
};
1461+
mockContext = { repo: { owner: "test-owner", repo: "test-repo" } };
1462+
mockExecGitSync = vi.fn();
1463+
mockFs = {
1464+
existsSync: vi.fn().mockReturnValue(false),
1465+
readFileSync: vi.fn(),
1466+
writeFileSync: vi.fn(),
1467+
mkdirSync: vi.fn(),
1468+
readdirSync: vi.fn(),
1469+
statSync: vi.fn(),
1470+
copyFileSync: vi.fn(),
1471+
};
1472+
1473+
global.core = mockCore;
1474+
global.context = mockContext;
1475+
1476+
process.env.ARTIFACT_DIR = "/tmp/test-artifact";
1477+
process.env.MEMORY_ID = "test-memory";
1478+
process.env.BRANCH_NAME = "memory/test";
1479+
process.env.GH_TOKEN = "test-token";
1480+
process.env.GITHUB_RUN_ID = "123456";
1481+
process.env.GITHUB_WORKSPACE = "/tmp/workspace";
1482+
process.env.TARGET_REPO = "test-owner/test-repo";
1483+
});
1484+
1485+
afterEach(() => {
1486+
delete global.core;
1487+
delete global.context;
1488+
delete process.env.ARTIFACT_DIR;
1489+
delete process.env.MEMORY_ID;
1490+
delete process.env.BRANCH_NAME;
1491+
delete process.env.GH_TOKEN;
1492+
delete process.env.GITHUB_RUN_ID;
1493+
delete process.env.GITHUB_WORKSPACE;
1494+
delete process.env.TARGET_REPO;
1495+
delete process.env.FILE_GLOB_FILTER;
1496+
vi.resetModules();
1497+
});
1498+
1499+
it("should reject FILE_GLOB_FILTER patterns starting with /", async () => {
1500+
process.env.FILE_GLOB_FILTER = "/etc/passwd";
1501+
1502+
vi.doMock("fs", () => mockFs);
1503+
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));
1504+
1505+
const { main } = await import("./push_repo_memory.cjs");
1506+
await main();
1507+
1508+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('"/etc/passwd"'));
1509+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("must not start with"));
1510+
expect(mockExecGitSync).not.toHaveBeenCalled();
1511+
});
1512+
1513+
it("should reject when any pattern in a multi-pattern filter starts with /", async () => {
1514+
process.env.FILE_GLOB_FILTER = "*.json /absolute/path.md";
1515+
1516+
vi.doMock("fs", () => mockFs);
1517+
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));
1518+
1519+
const { main } = await import("./push_repo_memory.cjs");
1520+
await main();
1521+
1522+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('"/absolute/path.md"'));
1523+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("must not start with"));
1524+
});
1525+
1526+
it("should accept valid slashless patterns without error", async () => {
1527+
process.env.FILE_GLOB_FILTER = "*.json *.md";
1528+
1529+
// Artifact dir doesn't exist → quick exit, no setFailed
1530+
mockFs.existsSync.mockReturnValue(false);
1531+
1532+
vi.doMock("fs", () => mockFs);
1533+
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));
1534+
1535+
const { main } = await import("./push_repo_memory.cjs");
1536+
await main();
1537+
1538+
expect(mockCore.setFailed).not.toHaveBeenCalled();
1539+
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Memory directory not found"));
1540+
});
1541+
1542+
it("should accept valid subfolder-scoped patterns without error", async () => {
1543+
process.env.FILE_GLOB_FILTER = "metrics/** data/**";
1544+
1545+
mockFs.existsSync.mockReturnValue(false);
1546+
1547+
vi.doMock("fs", () => mockFs);
1548+
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));
1549+
1550+
const { main } = await import("./push_repo_memory.cjs");
1551+
await main();
1552+
1553+
expect(mockCore.setFailed).not.toHaveBeenCalled();
1554+
});
1555+
});
1556+
14491557
describe("push_repo_memory.cjs - changed-file limit checks", () => {
14501558
it("should enforce MAX_FILE_COUNT against changed files, not scanned wiki file count (source check)", () => {
14511559
const nodeFs = require("fs");

docs/src/content/docs/reference/frontmatter-full.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3274,8 +3274,11 @@ tools:
32743274
# (optional)
32753275
branch-name: "example-value"
32763276

3277-
# Glob patterns for files to include in repository memory. Supports wildcards
3278-
# (e.g., '**/*.md', 'docs/**/*.json') to filter cached files.
3277+
# Glob patterns for files to include in repository memory. Slashless patterns
3278+
# (e.g., '*.json', '*.md') match files at the root of a single memory subfolder
3279+
# (depth 1 only). Patterns containing '/' (e.g., 'metrics/**', 'data/*.csv')
3280+
# match against the full relative path. Absolute paths starting with '/' are
3281+
# rejected. Do not include the branch name in patterns.
32793282
# (optional)
32803283
# Accepted formats:
32813284

docs/src/content/docs/reference/repo-memory.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,14 @@ tools:
4646

4747
**Patch Size Limit**: Use `max-patch-size` to limit the total size of changes in a single push (default: 10KB, max: 1MB). The total size of the git diff (all staged changes combined) must not exceed this value. If it does, the push is rejected with an error. Use this to prevent large unintentional memory updates.
4848

49-
**Note**: File glob patterns are matched against the **relative file path** within the artifact directory, not the branch path. Use bare extension patterns like `*.json` or `*.md` — do **not** include the branch name (e.g. `memory/custom-agent-for-aw/*.json` is incorrect).
49+
**File Glob Matching Rules**:
50+
51+
- Patterns are matched against the **relative path** within the artifact directory — do **not** include the branch name.
52+
- **Slashless patterns** (no `/` in the pattern, e.g. `*.json`, `*.md`) match files at the root of a single memory subfolder — **depth 1 only**. They do _not_ match files at the artifact root (depth 0) or deeper than one subfolder level (depth 2+).
53+
- **Patterns containing `/`** (e.g. `metrics/**`, `data/*.csv`) are matched against the full relative path from the artifact root and work as standard glob expressions.
54+
- **Absolute paths** (patterns starting with `/`) are **not supported** and are rejected at compile time and runtime.
55+
56+
Example: with the default filter `["*.json", "*.md"]`, the file `discussion-task-miner/processed-discussions.json` is persisted (depth 1 ✓), but `processed-discussions.json` (depth 0) and `discussion-task-miner/archive/old.json` (depth 2) are not.
5057

5158
## Multiple Configurations
5259

@@ -63,7 +70,7 @@ tools:
6370
---
6471
```
6572

66-
Mounts at `/tmp/gh-aw/repo-memory-{id}/` during workflow execution. Required `id` determines folder name; `branch-name` defaults to `{branch-prefix}/{id}` (where `branch-prefix` defaults to `memory`). Files are stored within the git branch at the branch name path (e.g., for branch `memory/code-metrics`, files are stored at `memory/code-metrics/` within the branch). **File glob patterns are matched against the relative file path within the artifact directory — never include the branch name in patterns.**
73+
Mounts at `/tmp/gh-aw/repo-memory-{id}/` during workflow execution. Required `id` determines folder name; `branch-name` defaults to `{branch-prefix}/{id}` (where `branch-prefix` defaults to `memory`). Files are stored within the git branch at the branch name path (e.g., for branch `memory/code-metrics`, files are stored at `memory/code-metrics/` within the branch). **File glob patterns are matched against the relative path within the artifact directory — never include the branch name in patterns. Slashless patterns (e.g. `*.json`) match files at the root of a memory subfolder (depth 1 only).**
6774

6875
## Behavior
6976

pkg/workflow/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,7 @@ Detects dangerous patterns in externally-sourced markdown files (e.g., from `gh
576576
|----------|-----------|-------------|
577577
| `ValidateEventFilters` | `func(map[string]any) error` | Validates `on:` event filter patterns |
578578
| `ValidateGlobPatterns` | `func(map[string]any) error` | Validates glob patterns in trigger filters |
579+
| `validateFileGlobPatterns` | `func([]string) error` | Validates `file-glob` patterns for a repo-memory entry; rejects absolute paths (starting with `/`) |
579580

580581
### Step Types
581582

0 commit comments

Comments
 (0)