Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion actions/setup/js/glob_pattern_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ function escapeRegexChars(pattern) {
* @param {Object} [options] - Options for pattern conversion
* @param {boolean} [options.pathMode=true] - If true, * matches non-slash chars; if false, * matches any char
* @param {boolean} [options.caseSensitive=true] - Whether matching should be case-sensitive
* @param {boolean} [options.matchSubfolderRoot=false] - If true and the pattern contains no "/",
* the pattern is required to be prefixed by exactly one path segment (i.e. matches only at the
* root of a single memory subfolder: "subfolder/file.json"). Files at the artifact root (depth 0)
* and files deeper than one level (depth 2+) are not matched.
* @returns {RegExp} - Regular expression that matches the pattern
*
* Supports:
Expand All @@ -37,9 +41,15 @@ function escapeRegexChars(pattern) {
* const regex = globPatternToRegex("metrics/**");
* regex.test("metrics/data.json"); // true
* regex.test("metrics/daily/data.json"); // true
*
* @example
* const regex = globPatternToRegex("*.json", { matchSubfolderRoot: true });
* regex.test("discussion-task-miner/processed-discussions.json"); // true (depth 1)
* regex.test("file.json"); // false (depth 0 – not matched)
* regex.test("discussion-task-miner/archive/old.json"); // false (depth 2 – not matched)
*/
function globPatternToRegex(pattern, options) {
const { pathMode = true, caseSensitive = true } = options || {};
const { pathMode = true, caseSensitive = true, matchSubfolderRoot = false } = options || {};

let regexPattern = escapeRegexChars(pattern);

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

// matchSubfolderRoot: slashless patterns must match exactly at the root of one subfolder
// (depth 1 only – neither depth 0 nor depth 2+).
// Patterns that already contain a "/" are matched against the full relative path unchanged.
if (matchSubfolderRoot && !pattern.includes("/")) {
regexPattern = `[^/]+/${regexPattern}`;
}

return new RegExp(`^${regexPattern}$`, caseSensitive ? "" : "i");
}

Expand Down
51 changes: 51 additions & 0 deletions actions/setup/js/glob_pattern_helpers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,57 @@ describe("glob_pattern_helpers.cjs", () => {
});
});

describe("matchSubfolderRoot option", () => {
it("slashless pattern matches only at subfolder root (depth 1)", () => {
const regex = globPatternToRegex("*.json", { matchSubfolderRoot: true });

// depth 1 – the intended match
expect(regex.test("discussion-task-miner/processed-discussions.json")).toBe(true);
expect(regex.test("subfolder/file.json")).toBe(true);

// depth 0 – must NOT match
expect(regex.test("file.json")).toBe(false);

// depth 2+ – must NOT match
expect(regex.test("discussion-task-miner/archive/old.json")).toBe(false);
expect(regex.test("a/b/c/file.json")).toBe(false);
});

it("slashless pattern with *.md matches at subfolder root only", () => {
const regex = globPatternToRegex("*.md", { matchSubfolderRoot: true });

expect(regex.test("discussion-task-miner/latest-run.md")).toBe(true);
expect(regex.test("latest-run.md")).toBe(false);
expect(regex.test("discussion-task-miner/docs/latest-run.md")).toBe(false);
});

it("pattern with slash is unaffected by matchSubfolderRoot", () => {
const regex = globPatternToRegex("metrics/**", { matchSubfolderRoot: true });

expect(regex.test("metrics/file.json")).toBe(true);
expect(regex.test("metrics/daily/file.json")).toBe(true);
expect(regex.test("other/file.json")).toBe(false);
});

it("default extension globs match discussion-task-miner layout", () => {
const patternStrs = ["*.json", "*.jsonl", "*.csv", "*.md"];
const patterns = patternStrs.map(p => globPatternToRegex(p, { matchSubfolderRoot: true }));

// discussion-task-miner state files (depth 1) should all match
expect(patterns.some(p => p.test("discussion-task-miner/processed-discussions.json"))).toBe(true);
expect(patterns.some(p => p.test("discussion-task-miner/latest-run.md"))).toBe(true);

// root-level files (depth 0) should NOT match
expect(patterns.some(p => p.test("processed-discussions.json"))).toBe(false);

// too deep (depth 2+) should NOT match
expect(patterns.some(p => p.test("discussion-task-miner/archive/old.json"))).toBe(false);

// non-matching extension should not match
expect(patterns.some(p => p.test("discussion-task-miner/script.js"))).toBe(false);
});
});

describe("simpleGlobToRegex", () => {
it("should match exact patterns without wildcards", () => {
const regex = simpleGlobToRegex("copilot");
Expand Down
76 changes: 44 additions & 32 deletions actions/setup/js/push_repo_memory.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ async function main() {
return;
}

// Validate FILE_GLOB_FILTER patterns: absolute paths (starting with "/") are not supported.
if (fileGlobFilter) {
const allPatternStrs = fileGlobFilter.trim().split(/\s+/).filter(Boolean);
for (const pat of allPatternStrs) {
if (pat.startsWith("/")) {
core.setFailed(`FILE_GLOB_FILTER contains an unsupported pattern: "${pat}". Patterns must not start with "/" (absolute paths are not allowed).`);
return;
}
}
}

// Source directory with memory files (artifact location)
// The artifactDir IS the memory directory (no nested structure needed)
const sourceMemoryPath = artifactDir;
Expand Down Expand Up @@ -274,12 +285,24 @@ async function main() {

// Recursively scan and collect files from artifact directory
let filesToCopy = [];

// Log the file glob filter configuration
/** @type {Array<{path: string, reason: string}>} */
let filteredOutFiles = [];

// Compile glob patterns once, outside the scan loop
/** @type {string[]} */
let patternStrs = [];
/** @type {RegExp[]} */
let compiledPatterns = [];
if (fileGlobFilter) {
core.info(`File glob filter enabled: ${fileGlobFilter}`);
const patternCount = fileGlobFilter.trim().split(/\s+/).filter(Boolean).length;
core.info(`Number of patterns: ${patternCount}`);
patternStrs = fileGlobFilter.trim().split(/\s+/).filter(Boolean);
// Slashless patterns (e.g. "*.json") are matched at the root of a single memory subfolder
// (depth 1 only). Patterns that already contain "/" are matched against the full relative
// path unchanged.
compiledPatterns = patternStrs.map(pattern => globPatternToRegex(pattern, { matchSubfolderRoot: !pattern.includes("/") }));
core.info(`File glob filter enabled with ${patternStrs.length} pattern(s):`);
patternStrs.forEach((pat, idx) => {
core.info(` [${idx + 1}] "${pat}" -> regex: ${compiledPatterns[idx].source}`);
});
} else {
core.info("No file glob filter - all files will be accepted");
}
Expand All @@ -297,41 +320,24 @@ async function main() {
const relativeFilePath = relativePath ? path.join(relativePath, entry.name) : entry.name;

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

// Validate file name patterns if filter is set
if (fileGlobFilter) {
const patternStrs = fileGlobFilter.trim().split(/\s+/).filter(Boolean);
const patterns = patternStrs.map(pattern => globPatternToRegex(pattern));

// Test patterns against the relative file path within the memory directory
// Patterns are specified relative to the memory artifact directory, not the branch path
const normalizedRelPath = relativeFilePath.replace(/\\/g, "/");

// Enhanced logging: Show what we're testing (use info for first file to aid debugging)
core.debug(`Testing file: ${normalizedRelPath}`);
core.debug(`File glob filter: ${fileGlobFilter}`);
core.debug(`Number of patterns: ${patterns.length}`);

const matchResults = patterns.map((pattern, idx) => {
if (compiledPatterns.length > 0) {
const matchResults = compiledPatterns.map((pattern, idx) => {
const matches = pattern.test(normalizedRelPath);
core.debug(` Pattern ${idx + 1}: "${patternStrs[idx]}" -> ${pattern.source} -> ${matches ? "✓ MATCH" : "✗ NO MATCH"}`);
core.info(` [test] ${normalizedRelPath} pattern[${idx + 1}] "${patternStrs[idx]}" -> ${matches ? "✓ match" : "✗ no match"}`);
return matches;
});

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

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

try {
core.info(`Scanning artifact directory: ${sourceMemoryPath}`);
scanDirectory(sourceMemoryPath);
core.info(`Scan complete: Found ${filesToCopy.length} file(s) to copy`);
core.info(`Scan complete: ${filesToCopy.length} file(s) accepted, ${filteredOutFiles.length} file(s) filtered out`);
if (filteredOutFiles.length > 0) {
core.info(`Filtered-out files (${filteredOutFiles.length}):`);
filteredOutFiles.forEach(f => core.info(` - ${f.path} (${f.reason})`));
}
if (filesToCopy.length > 0 && filesToCopy.length <= 10) {
core.info("Files found:");
core.info("Accepted files:");
filesToCopy.forEach(f => core.info(` - ${f.relativePath} (${f.size} bytes)`));
} else if (filesToCopy.length > 10) {
core.info(`First 10 files:`);
core.info(`Accepted files (first 10 of ${filesToCopy.length}):`);
filesToCopy.slice(0, 10).forEach(f => core.info(` - ${f.relativePath} (${f.size} bytes)`));
core.info(` ... and ${filesToCopy.length - 10} more`);
}
Expand Down
108 changes: 108 additions & 0 deletions actions/setup/js/push_repo_memory.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,114 @@ describe("push_repo_memory.cjs - shell injection security tests", () => {
});
});

describe("push_repo_memory.cjs - FILE_GLOB_FILTER validation", () => {
let mockCore, mockContext, mockFs, mockExecGitSync;

beforeEach(() => {
mockCore = {
info: vi.fn(),
warning: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
setFailed: vi.fn(),
setOutput: vi.fn(),
};
mockContext = { repo: { owner: "test-owner", repo: "test-repo" } };
mockExecGitSync = vi.fn();
mockFs = {
existsSync: vi.fn().mockReturnValue(false),
readFileSync: vi.fn(),
writeFileSync: vi.fn(),
mkdirSync: vi.fn(),
readdirSync: vi.fn(),
statSync: vi.fn(),
copyFileSync: vi.fn(),
};

global.core = mockCore;
global.context = mockContext;

process.env.ARTIFACT_DIR = "/tmp/test-artifact";
process.env.MEMORY_ID = "test-memory";
process.env.BRANCH_NAME = "memory/test";
process.env.GH_TOKEN = "test-token";
process.env.GITHUB_RUN_ID = "123456";
process.env.GITHUB_WORKSPACE = "/tmp/workspace";
process.env.TARGET_REPO = "test-owner/test-repo";
});

afterEach(() => {
delete global.core;
delete global.context;
delete process.env.ARTIFACT_DIR;
delete process.env.MEMORY_ID;
delete process.env.BRANCH_NAME;
delete process.env.GH_TOKEN;
delete process.env.GITHUB_RUN_ID;
delete process.env.GITHUB_WORKSPACE;
delete process.env.TARGET_REPO;
delete process.env.FILE_GLOB_FILTER;
vi.resetModules();
});

it("should reject FILE_GLOB_FILTER patterns starting with /", async () => {
process.env.FILE_GLOB_FILTER = "/etc/passwd";

vi.doMock("fs", () => mockFs);
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));

const { main } = await import("./push_repo_memory.cjs");
await main();

expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('"/etc/passwd"'));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("must not start with"));
expect(mockExecGitSync).not.toHaveBeenCalled();
});

it("should reject when any pattern in a multi-pattern filter starts with /", async () => {
process.env.FILE_GLOB_FILTER = "*.json /absolute/path.md";

vi.doMock("fs", () => mockFs);
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));

const { main } = await import("./push_repo_memory.cjs");
await main();

expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining('"/absolute/path.md"'));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("must not start with"));
});

it("should accept valid slashless patterns without error", async () => {
process.env.FILE_GLOB_FILTER = "*.json *.md";

// Artifact dir doesn't exist → quick exit, no setFailed
mockFs.existsSync.mockReturnValue(false);

vi.doMock("fs", () => mockFs);
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));

const { main } = await import("./push_repo_memory.cjs");
await main();

expect(mockCore.setFailed).not.toHaveBeenCalled();
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Memory directory not found"));
});

it("should accept valid subfolder-scoped patterns without error", async () => {
process.env.FILE_GLOB_FILTER = "metrics/** data/**";

mockFs.existsSync.mockReturnValue(false);

vi.doMock("fs", () => mockFs);
vi.doMock("./git_helpers.cjs", () => ({ execGitSync: mockExecGitSync }));

const { main } = await import("./push_repo_memory.cjs");
await main();

expect(mockCore.setFailed).not.toHaveBeenCalled();
});
});

describe("push_repo_memory.cjs - changed-file limit checks", () => {
it("should enforce MAX_FILE_COUNT against changed files, not scanned wiki file count (source check)", () => {
const nodeFs = require("fs");
Expand Down
7 changes: 5 additions & 2 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -3274,8 +3274,11 @@ tools:
# (optional)
branch-name: "example-value"

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

Expand Down
11 changes: 9 additions & 2 deletions docs/src/content/docs/reference/repo-memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ tools:

**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.

**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).
**File Glob Matching Rules**:

- Patterns are matched against the **relative path** within the artifact directory — do **not** include the branch name.
- **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+).
- **Patterns containing `/`** (e.g. `metrics/**`, `data/*.csv`) are matched against the full relative path from the artifact root and work as standard glob expressions.
- **Absolute paths** (patterns starting with `/`) are **not supported** and are rejected at compile time and runtime.

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.

## Multiple Configurations

Expand All @@ -63,7 +70,7 @@ tools:
---
```

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.**
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).**

## Behavior

Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ Detects dangerous patterns in externally-sourced markdown files (e.g., from `gh
|----------|-----------|-------------|
| `ValidateEventFilters` | `func(map[string]any) error` | Validates `on:` event filter patterns |
| `ValidateGlobPatterns` | `func(map[string]any) error` | Validates glob patterns in trigger filters |
| `validateFileGlobPatterns` | `func([]string) error` | Validates `file-glob` patterns for a repo-memory entry; rejects absolute paths (starting with `/`) |

### Step Types

Expand Down
Loading
Loading