Skip to content

Commit 1371073

Browse files
Copilotgh-aw-bot
Copilot
and
gh-aw-bot
authored
Filter repo-memory validation to eligible files
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
1 parent f91aecd commit 1371073

5 files changed

Lines changed: 189 additions & 18 deletions

File tree

‎actions/setup/js/git_patch_utils.cjs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,18 @@ function getPatchDiffSizeBytes(patchContent) {
165165
* @param {Object} options
166166
* @param {(args: string[], opts?: Record<string, any>) => string} options.execGitSyncFn
167167
* @param {string} [options.cwd]
168+
* @param {string[]} [options.pathspecs] - Optional pathspecs to limit the staged diff
168169
* @returns {number}
169170
*/
170-
function getStagedPatchDiffSizeBytes({ execGitSyncFn, cwd }) {
171-
const patchContent = execGitSyncFn(["diff", "--cached"], { stdio: "pipe", cwd });
171+
function getStagedPatchDiffSizeBytes({ execGitSyncFn, cwd, pathspecs }) {
172+
if (Array.isArray(pathspecs) && pathspecs.length === 0) {
173+
return 0;
174+
}
175+
const args = ["diff", "--cached", "--no-renames"];
176+
if (Array.isArray(pathspecs)) {
177+
args.push("--", ...pathspecs);
178+
}
179+
const patchContent = execGitSyncFn(args, { stdio: "pipe", cwd });
172180
return getPatchDiffSizeBytes(patchContent);
173181
}
174182

‎actions/setup/js/git_patch_utils.test.cjs‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ describe("getStagedPatchDiffSizeBytes", () => {
358358
const diffOutput = ["diff --git a/data.json b/data.json", "index abc..def 100644", "--- a/data.json", "+++ b/data.json", "@@ -1 +1,2 @@", '-{"old":1}', '+{"new":1}', '+{"extra":2}'].join("\n");
359359

360360
const execGitSyncFn = (/** @type {string[]} */ args, /** @type {any} */ _opts) => {
361-
if (args[0] === "diff" && args[1] === "--cached") return diffOutput;
361+
if (args[0] === "diff" && args[1] === "--cached" && !args.includes("--")) return diffOutput;
362362
return "";
363363
};
364364

@@ -377,9 +377,28 @@ describe("getStagedPatchDiffSizeBytes", () => {
377377

378378
getStagedPatchDiffSizeBytes({ execGitSyncFn, cwd: "/memory/dir" });
379379
expect(calls).toHaveLength(1);
380-
expect(calls[0].args).toEqual(["diff", "--cached"]);
380+
expect(calls[0].args).toEqual(["diff", "--cached", "--no-renames"]);
381381
expect(calls[0].opts.cwd).toBe("/memory/dir");
382382
});
383+
384+
it("limits patch size to the supplied pathspecs", () => {
385+
const calls = /** @type {string[][]} */ [];
386+
const execGitSyncFn = (/** @type {string[]} */ args, /** @type {any} */ _opts) => {
387+
calls.push(args);
388+
return "";
389+
};
390+
391+
getStagedPatchDiffSizeBytes({ execGitSyncFn, cwd: "/memory/dir", pathspecs: [":(literal)state.json"] });
392+
393+
expect(calls).toEqual([["diff", "--cached", "--no-renames", "--", ":(literal)state.json"]]);
394+
});
395+
396+
it("returns zero without running git when the pathspec list is empty", () => {
397+
const execGitSyncFn = vi.fn();
398+
399+
expect(getStagedPatchDiffSizeBytes({ execGitSyncFn, cwd: "/memory/dir", pathspecs: [] })).toBe(0);
400+
expect(execGitSyncFn).not.toHaveBeenCalled();
401+
});
383402
});
384403

385404
describe("isAncestorCommit", () => {

‎actions/setup/js/memory_custom_validation.cjs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ function boundedOutput(output) {
112112
/**
113113
* @param {string} dirPath
114114
* @param {number} maxFileSize
115+
* @param {(relativePath: string) => boolean} [isEligibleFile]
115116
*/
116-
function formatJSONFiles(dirPath, maxFileSize) {
117+
function formatJSONFiles(dirPath, maxFileSize, isEligibleFile = () => true) {
117118
if (!fs.existsSync(dirPath)) {
118119
return [];
119120
}
@@ -122,18 +123,20 @@ function formatJSONFiles(dirPath, maxFileSize) {
122123

123124
/**
124125
* @param {string} currentDir
126+
* @param {string} relativePath
125127
*/
126-
function visit(currentDir) {
128+
function visit(currentDir, relativePath) {
127129
const entries = readDirectory(currentDir);
128130
for (const entry of entries) {
129131
const fullPath = path.join(currentDir, entry.name);
132+
const relativeFilePath = relativePath ? path.join(relativePath, entry.name) : entry.name;
130133
if (entry.isDirectory()) {
131134
if (entry.name !== ".git") {
132-
visit(fullPath);
135+
visit(fullPath, relativeFilePath);
133136
}
134137
continue;
135138
}
136-
if (!entry.isFile() || !entry.name.endsWith(".json")) {
139+
if (!entry.isFile() || !entry.name.endsWith(".json") || !isEligibleFile(relativeFilePath.replace(/\\/g, "/"))) {
137140
continue;
138141
}
139142
const raw = readFile(fullPath, "utf8");
@@ -159,7 +162,7 @@ function formatJSONFiles(dirPath, maxFileSize) {
159162
}
160163
}
161164

162-
visit(dirPath);
165+
visit(dirPath, "");
163166
return formattedFiles;
164167
}
165168

‎actions/setup/js/safe_outputs_handlers.cjs‎

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,58 @@ const { resolveDataSchema } = require("./data_schema_normalizer.cjs");
3434
const { clearValidationMarker, formatJSONFiles, runCustomMemoryValidation, writeValidationMarker } = require("./memory_custom_validation.cjs");
3535
const { compileFileGlobPatterns, isMemoryFileEligible } = require("./memory_file_eligibility.cjs");
3636

37+
function createEligibleMemoryValidationView(memoryDir, isEligibleFile) {
38+
let validationDir;
39+
try {
40+
validationDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-repo-memory-validation-"));
41+
} catch (error) {
42+
throw new Error(`${ERR_SYSTEM}: Failed to create repo-memory validation directory: ${getErrorMessage(error)}`, { cause: error });
43+
}
44+
45+
/**
46+
* @param {string} targetPath
47+
*/
48+
function removeValidationDirectory(targetPath) {
49+
try {
50+
fs.rmSync(targetPath, { recursive: true, force: true });
51+
} catch (error) {
52+
throw new Error(`${ERR_SYSTEM}: Failed to remove repo-memory validation directory: ${getErrorMessage(error)}`, { cause: error });
53+
}
54+
}
55+
56+
try {
57+
/**
58+
* @param {string} sourceDir
59+
* @param {string} relativePath
60+
*/
61+
function copyEligibleFiles(sourceDir, relativePath) {
62+
for (const entry of fs.readdirSync(sourceDir, { withFileTypes: true })) {
63+
const sourcePath = path.join(sourceDir, entry.name);
64+
const relativeFilePath = relativePath ? path.join(relativePath, entry.name) : entry.name;
65+
if (entry.isDirectory()) {
66+
if (entry.name !== ".git") {
67+
copyEligibleFiles(sourcePath, relativeFilePath);
68+
}
69+
} else if (entry.isFile() && isEligibleFile(relativeFilePath)) {
70+
const targetPath = path.join(validationDir, relativeFilePath);
71+
fs.mkdirSync(path.dirname(targetPath), { recursive: true });
72+
fs.copyFileSync(sourcePath, targetPath);
73+
}
74+
}
75+
}
76+
77+
copyEligibleFiles(memoryDir, "");
78+
return validationDir;
79+
} catch (error) {
80+
try {
81+
removeValidationDirectory(validationDir);
82+
} catch (cleanupError) {
83+
throw new Error(`${ERR_SYSTEM}: Failed to create repo-memory validation view: ${getErrorMessage(error)}; cleanup also failed: ${getErrorMessage(cleanupError)}`, { cause: error });
84+
}
85+
throw error;
86+
}
87+
}
88+
3789
/** PR event names used for target:triggering context validation across all safe-output handlers. */
3890
const PR_EVENT_NAMES = new Set(["pull_request", "pull_request_target", "pull_request_review", "pull_request_review_comment"]);
3991

@@ -1849,7 +1901,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
18491901

18501902
if (memoryConf.format_json === true) {
18511903
try {
1852-
const formattedFiles = formatJSONFiles(memoryDir, maxFileSize);
1904+
const formattedFiles = formatJSONFiles(memoryDir, maxFileSize, isEligibleFile);
18531905
if (formattedFiles.length > 0) {
18541906
core.info(`Formatted ${formattedFiles.length} repo-memory JSON file(s) before validation: ${formattedFiles.join(", ")}`);
18551907
}
@@ -1971,12 +2023,14 @@ function createHandlers(server, appendSafeOutput, config = {}) {
19712023
let patchSizeBytes;
19722024
try {
19732025
ensureSafeDirectoryTrust(memoryDir, server);
2026+
const stagedFiles = execGitSync(["diff", "--cached", "--no-renames", "--name-only", "-z"], { cwd: memoryDir, stdio: "pipe" }).split("\0").filter(Boolean).filter(isEligibleFile);
19742027
const trackedFiles = execGitSync(["ls-files", "-z"], { cwd: memoryDir, stdio: "pipe" }).split("\0").filter(Boolean).filter(isEligibleFile);
19752028
const filesToStage = [...new Set([...files.map(file => file.relativePath), ...trackedFiles])];
19762029
if (filesToStage.length > 0) {
19772030
execGitSync(["add", "--sparse", "--all", "--", ...filesToStage.map(file => `:(literal)${file}`)], { cwd: memoryDir, stdio: "pipe" });
19782031
}
1979-
patchSizeBytes = getStagedPatchDiffSizeBytes({ execGitSyncFn: execGitSync, cwd: memoryDir });
2032+
const filesToMeasure = [...new Set([...stagedFiles, ...filesToStage])].map(file => `:(literal)${file}`);
2033+
patchSizeBytes = getStagedPatchDiffSizeBytes({ execGitSyncFn: execGitSync, cwd: memoryDir, pathspecs: filesToMeasure });
19802034
} catch (/** @type {any} */ error) {
19812035
return {
19822036
content: [
@@ -2017,13 +2071,22 @@ function createHandlers(server, appendSafeOutput, config = {}) {
20172071
/** @type {ReturnType<typeof runCustomMemoryValidation> | null} */
20182072
let customValidation = null;
20192073
if (validationConfig) {
2020-
customValidation = runCustomMemoryValidation({
2021-
script: validationScript,
2022-
memoryDir,
2023-
memoryId,
2024-
kind: "repo",
2025-
timeoutSeconds: validationTimeoutSeconds,
2026-
});
2074+
const validationDir = createEligibleMemoryValidationView(memoryDir, isEligibleFile);
2075+
try {
2076+
customValidation = runCustomMemoryValidation({
2077+
script: validationScript,
2078+
memoryDir: validationDir,
2079+
memoryId,
2080+
kind: "repo",
2081+
timeoutSeconds: validationTimeoutSeconds,
2082+
});
2083+
} finally {
2084+
try {
2085+
fs.rmSync(validationDir, { recursive: true, force: true });
2086+
} catch (error) {
2087+
throw new Error(`${ERR_SYSTEM}: Failed to clean up repo-memory validation directory: ${getErrorMessage(error)}`, { cause: error });
2088+
}
2089+
}
20272090
if (!customValidation.ok) {
20282091
const reason = customValidation.timedOut ? `timed out after ${validationTimeoutSeconds || 30} second(s)` : `exited with code ${customValidation.exitCode}`;
20292092
return {

‎actions/setup/js/safe_outputs_handlers.test.cjs‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3401,6 +3401,84 @@ describe("safe_outputs_handlers", () => {
34013401
}
34023402
});
34033403

3404+
it("should format and custom-validate only eligible JSON files", () => {
3405+
const legacyJson = JSON.stringify(Object.fromEntries(Array.from({ length: 8 }, (_, index) => [`k${index}`, "x"])));
3406+
const maxFileSize = Buffer.byteLength(legacyJson, "utf8") + 1;
3407+
expect(Buffer.byteLength(`${JSON.stringify(JSON.parse(legacyJson), null, 2)}\n`, "utf8")).toBeGreaterThan(maxFileSize);
3408+
const malformedJson = "{invalid";
3409+
const h = makeHandlersWithMemory({
3410+
file_glob: "state.json",
3411+
format_json: true,
3412+
max_file_size: maxFileSize,
3413+
max_file_count: 1,
3414+
validation: {
3415+
script: `
3416+
const jsonFiles = fs.readdirSync(memoryRoot).filter(file => file.endsWith(".json"));
3417+
if (jsonFiles.length !== 1 || jsonFiles[0] !== "state.json") throw new Error("unexpected JSON files in validation view");
3418+
if (memoryRoot !== process.cwd()) throw new Error("validation working directory does not match memory root");
3419+
JSON.parse(fs.readFileSync("state.json", "utf8"));
3420+
`,
3421+
timeout: 5,
3422+
},
3423+
});
3424+
fs.mkdirSync(memoryDir, { recursive: true });
3425+
initGitRepo(memoryDir);
3426+
fs.writeFileSync(path.join(memoryDir, "state.json"), '{"ok":true}');
3427+
fs.writeFileSync(path.join(memoryDir, "legacy.json"), legacyJson);
3428+
fs.writeFileSync(path.join(memoryDir, "malformed.json"), malformedJson);
3429+
3430+
const result = h.pushRepoMemoryHandler({ memory_id: "default" });
3431+
const data = JSON.parse(result.content[0].text);
3432+
3433+
expect(result.isError).toBeUndefined();
3434+
expect(data.result).toBe("success");
3435+
expect(data.storage_validation.files).toBe(1);
3436+
expect(data.custom_validation.result).toBe("success");
3437+
expect(fs.readFileSync(path.join(memoryDir, "state.json"), "utf8")).toBe('{\n "ok": true\n}\n');
3438+
expect(fs.readFileSync(path.join(memoryDir, "legacy.json"), "utf8")).toBe(legacyJson);
3439+
expect(fs.readFileSync(path.join(memoryDir, "malformed.json"), "utf8")).toBe(malformedJson);
3440+
});
3441+
3442+
it("should keep custom validation read-only when using the filtered view", () => {
3443+
const originalState = '{"ok":true}';
3444+
const h = makeHandlersWithMemory({
3445+
file_glob: "state.json",
3446+
validation: {
3447+
script: `fs.writeFileSync(path.join(memoryRoot, "state.json"), '{"ok":false}');`,
3448+
timeout: 5,
3449+
},
3450+
});
3451+
fs.mkdirSync(memoryDir, { recursive: true });
3452+
initGitRepo(memoryDir);
3453+
fs.writeFileSync(path.join(memoryDir, "state.json"), originalState);
3454+
3455+
const result = h.pushRepoMemoryHandler({ memory_id: "default" });
3456+
const data = JSON.parse(result.content[0].text);
3457+
3458+
expect(result.isError).toBe(true);
3459+
expect(data.custom_validation.stderr).toContain("must not modify memory files");
3460+
expect(fs.readFileSync(path.join(memoryDir, "state.json"), "utf8")).toBe(originalState);
3461+
});
3462+
3463+
it("should measure only eligible staged files without changing excluded index entries", () => {
3464+
const h = makeHandlersWithMemory({ file_glob: "state.json", max_file_size: 1024, max_file_count: 1, max_patch_size: 10 });
3465+
fs.mkdirSync(memoryDir, { recursive: true });
3466+
initGitRepo(memoryDir);
3467+
fs.writeFileSync(path.join(memoryDir, "state.json"), "{}");
3468+
fs.writeFileSync(path.join(memoryDir, "legacy.json"), "x".repeat(1000));
3469+
execSync("git add legacy.json", { cwd: memoryDir, stdio: "pipe" });
3470+
const stagedLegacyBefore = execSync("git show :legacy.json", { cwd: memoryDir, encoding: "utf8" });
3471+
3472+
const result = h.pushRepoMemoryHandler({ memory_id: "default" });
3473+
const data = JSON.parse(result.content[0].text);
3474+
3475+
expect(result.isError).toBeUndefined();
3476+
expect(data.result).toBe("success");
3477+
expect(data.storage_validation.patch_size_bytes).toBeLessThanOrEqual(12);
3478+
expect(execSync("git show :legacy.json", { cwd: memoryDir, encoding: "utf8" })).toBe(stagedLegacyBefore);
3479+
expect(execSync("git diff --cached --name-only", { cwd: memoryDir, encoding: "utf8" }).trim().split("\n")).toEqual(["legacy.json", "state.json"]);
3480+
});
3481+
34043482
it("should pass when total folder size is large but staged diff is tiny", () => {
34053483
const h = makeHandlersWithMemory({ max_patch_size: 50, max_file_size: 1024 * 1024 });
34063484
fs.mkdirSync(memoryDir, { recursive: true });

0 commit comments

Comments
 (0)