diff --git a/actions/setup/js/glob_pattern_helpers.cjs b/actions/setup/js/glob_pattern_helpers.cjs index 1bdfed3f368..416470b0994 100644 --- a/actions/setup/js/glob_pattern_helpers.cjs +++ b/actions/setup/js/glob_pattern_helpers.cjs @@ -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: @@ -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); @@ -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"); } diff --git a/actions/setup/js/glob_pattern_helpers.test.cjs b/actions/setup/js/glob_pattern_helpers.test.cjs index b1633108e6a..b2a0bff986d 100644 --- a/actions/setup/js/glob_pattern_helpers.test.cjs +++ b/actions/setup/js/glob_pattern_helpers.test.cjs @@ -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"); diff --git a/actions/setup/js/push_repo_memory.cjs b/actions/setup/js/push_repo_memory.cjs index 7c0162ef271..8653ef6e2fe 100644 --- a/actions/setup/js/push_repo_memory.cjs +++ b/actions/setup/js/push_repo_memory.cjs @@ -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; @@ -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"); } @@ -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; } @@ -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, @@ -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`); } diff --git a/actions/setup/js/push_repo_memory.test.cjs b/actions/setup/js/push_repo_memory.test.cjs index 12610f386ef..a3cee4f1614 100644 --- a/actions/setup/js/push_repo_memory.test.cjs +++ b/actions/setup/js/push_repo_memory.test.cjs @@ -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"); diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index c09f26b4747..8212f91f0b3 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -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: diff --git a/docs/src/content/docs/reference/repo-memory.md b/docs/src/content/docs/reference/repo-memory.md index a10751c7ca9..56e1769f33d 100644 --- a/docs/src/content/docs/reference/repo-memory.md +++ b/docs/src/content/docs/reference/repo-memory.md @@ -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 @@ -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 diff --git a/pkg/workflow/README.md b/pkg/workflow/README.md index 1cc8b941413..24bb1dcc233 100644 --- a/pkg/workflow/README.md +++ b/pkg/workflow/README.md @@ -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 diff --git a/pkg/workflow/repo_memory.go b/pkg/workflow/repo_memory.go index ec2ed0dec95..bce79c255c6 100644 --- a/pkg/workflow/repo_memory.go +++ b/pkg/workflow/repo_memory.go @@ -218,6 +218,9 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig, workflowID // Allow single string to be treated as array of one entry.FileGlob = []string{globStr} } + if err := validateFileGlobPatterns(entry.FileGlob); err != nil { + return nil, err + } } // Parse max-file-size @@ -382,6 +385,9 @@ func (c *Compiler) extractRepoMemoryConfig(toolsConfig *ToolsConfig, workflowID // Allow single string to be treated as array of one entry.FileGlob = []string{globStr} } + if err := validateFileGlobPatterns(entry.FileGlob); err != nil { + return nil, err + } } // Parse max-file-size diff --git a/pkg/workflow/repo_memory_test.go b/pkg/workflow/repo_memory_test.go index a53841e54f1..d12f68534b6 100644 --- a/pkg/workflow/repo_memory_test.go +++ b/pkg/workflow/repo_memory_test.go @@ -1544,3 +1544,122 @@ func TestRepoMemoryFormatJSONPushStepEnvVar(t *testing.T) { "Push step should NOT include FORMAT_JSON env var when format-json is false") }) } + +// TestValidateFileGlobPatterns tests the validateFileGlobPatterns function +func TestValidateFileGlobPatterns(t *testing.T) { + tests := []struct { + name string + patterns []string + wantErr bool + errMsg string + }{ + { + name: "empty patterns list", + patterns: []string{}, + wantErr: false, + }, + { + name: "valid slashless extension glob", + patterns: []string{"*.json", "*.md"}, + wantErr: false, + }, + { + name: "valid subfolder-scoped pattern", + patterns: []string{"metrics/**"}, + wantErr: false, + }, + { + name: "valid mixed patterns", + patterns: []string{"*.json", "*.jsonl", "*.csv", "*.md"}, + wantErr: false, + }, + { + name: "valid depth-1 exact pattern", + patterns: []string{"discussion-task-miner/processed-discussions.json"}, + wantErr: false, + }, + { + name: "invalid - pattern starts with /", + patterns: []string{"/etc/passwd"}, + wantErr: true, + errMsg: "must not start with '/'", + }, + { + name: "invalid - absolute path /file.json", + patterns: []string{"/file.json"}, + wantErr: true, + errMsg: "must not start with '/'", + }, + { + name: "invalid - second pattern starts with /", + patterns: []string{"*.json", "/absolute/path.md"}, + wantErr: true, + errMsg: "must not start with '/'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateFileGlobPatterns(tt.patterns) + if tt.wantErr { + require.Error(t, err, "Expected error for patterns: %v", tt.patterns) + assert.Contains(t, err.Error(), tt.errMsg, "Error message should contain: %s", tt.errMsg) + } else { + require.NoError(t, err, "Expected no error for patterns: %v", tt.patterns) + } + }) + } +} + +// TestFileGlobPatternValidationInConfig tests that invalid file-glob patterns are rejected +// during config extraction (both array and object notation) +func TestFileGlobPatternValidationInConfig(t *testing.T) { + t.Run("rejects absolute path in array notation", func(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": []any{ + map[string]any{ + "id": "test", + "file-glob": []any{"/etc/secret.json"}, + }, + }, + } + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err) + compiler := NewCompiler() + _, err = compiler.extractRepoMemoryConfig(toolsConfig, "test-workflow") + require.Error(t, err) + assert.Contains(t, err.Error(), "must not start with '/'") + }) + + t.Run("rejects absolute path in object notation", func(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": map[string]any{ + "file-glob": []any{"/absolute/path.md"}, + }, + } + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err) + compiler := NewCompiler() + _, err = compiler.extractRepoMemoryConfig(toolsConfig, "test-workflow") + require.Error(t, err) + assert.Contains(t, err.Error(), "must not start with '/'") + }) + + t.Run("accepts valid slashless patterns in array notation", func(t *testing.T) { + toolsMap := map[string]any{ + "repo-memory": []any{ + map[string]any{ + "id": "test", + "file-glob": []any{"*.json", "*.md"}, + }, + }, + } + toolsConfig, err := ParseToolsConfig(toolsMap) + require.NoError(t, err) + compiler := NewCompiler() + config, err := compiler.extractRepoMemoryConfig(toolsConfig, "test-workflow") + require.NoError(t, err) + require.NotNil(t, config) + assert.Equal(t, []string{"*.json", "*.md"}, config.Memories[0].FileGlob) + }) +} diff --git a/pkg/workflow/repo_memory_validation.go b/pkg/workflow/repo_memory_validation.go index ba5084dfd3c..282b09c7bae 100644 --- a/pkg/workflow/repo_memory_validation.go +++ b/pkg/workflow/repo_memory_validation.go @@ -9,6 +9,7 @@ // // - validateBranchPrefix() - Validates branch prefix length, format, and reserved names // - validateNoDuplicateMemoryIDs() - Ensures each memory entry has a unique ID +// - validateFileGlobPatterns() - Validates file-glob patterns for unsupported forms // // # When to Add Validation Here // @@ -36,21 +37,21 @@ func validateBranchPrefix(prefix string) error { // Check length (4-32 characters) if len(prefix) < 4 { - return fmt.Errorf("branch-prefix must be at least 4 characters long, got %d", len(prefix)) + return fmt.Errorf("branch-prefix must be at least 4 characters long, got %d. Example: branch-prefix: my-bot", len(prefix)) } if len(prefix) > 32 { - return fmt.Errorf("branch-prefix must be at most 32 characters long, got %d", len(prefix)) + return fmt.Errorf("branch-prefix must be at most 32 characters long, got %d. Example: branch-prefix: my-bot", len(prefix)) } // Check for alphanumeric and branch-friendly characters (alphanumeric, hyphens, underscores) // Use pre-compiled regex from package level for performance if !branchPrefixValidPattern.MatchString(prefix) { - return fmt.Errorf("branch-prefix must contain only alphanumeric characters, hyphens, and underscores, got '%s'", prefix) + return fmt.Errorf("branch-prefix must contain only alphanumeric characters, hyphens, and underscores, got '%s'. Example: branch-prefix: my-bot", prefix) } // Cannot be "copilot" if strings.EqualFold(prefix, "copilot") { - return errors.New("branch-prefix cannot be 'copilot' (reserved)") + return errors.New("branch-prefix cannot be 'copilot' (reserved). Example: branch-prefix: my-bot") } repoMemValidationLog.Printf("Branch prefix %q passed validation", prefix) @@ -62,6 +63,24 @@ func validateBranchPrefix(prefix string) error { func validateNoDuplicateMemoryIDs(memories []RepoMemoryEntry) error { repoMemValidationLog.Printf("Validating %d memory entries for duplicate IDs", len(memories)) return validateNoDuplicateIDs(memories, func(m RepoMemoryEntry) string { return m.ID }, func(id string) error { - return fmt.Errorf("duplicate memory ID found: '%s'. Each memory must have a unique ID", id) + return fmt.Errorf("duplicate memory ID found: '%s'. Each memory must have a unique ID. Example: id: my-memory", id) }) } + +// validateFileGlobPatterns validates file-glob patterns for a repo-memory entry. +// +// Patterns are evaluated relative to the memory subfolder root (depth 1 from the artifact root). +// Slashless patterns such as "*.json" match files at the root of any single memory subfolder. +// Patterns containing "/" match against the full relative path from the artifact root. +// +// Rejected patterns: +// - Patterns starting with "/" — absolute paths are not supported. +func validateFileGlobPatterns(patterns []string) error { + for _, pat := range patterns { + repoMemValidationLog.Printf("Validating file-glob pattern: %q", pat) + if strings.HasPrefix(pat, "/") { + return fmt.Errorf("file-glob pattern %q is not supported: patterns must not start with '/' (absolute paths are not allowed). Example: file-glob: [\"*.json\", \"*.md\"]", pat) + } + } + return nil +}