Skip to content

Commit 98e750c

Browse files
lpcoxCopilot
andcommitted
fix: address review feedback on cache-memory hardening
- Guard find with directory existence check to avoid failures when cache dir is missing (e.g., when prior setup step failed) - Preserve directory structure in quarantine instead of flattening paths, avoiding filename collisions and evidence loss - Escape head output with sed to prevent GitHub Actions workflow command injection from malicious file content - Apply cache key and restore-keys TTL transformations independently so partially updated workflows are correctly repaired Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1f877f4 commit 98e750c

10 files changed

+112
-56
lines changed

.github/workflows/ci-doctor.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/issue-duplication-detector.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/pelis-agent-factory-advisor.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/secret-digger-claude.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/secret-digger-codex.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/secret-digger-copilot.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/security-review.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-codex.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/smoke-services.lock.yml

Lines changed: 9 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/ci/postprocess-smoke-workflows.ts

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,12 @@ function buildStripExecBitsStep(indent: string): string {
147147
`${ri}CACHE_DIR="\${GH_AW_CACHE_DIR:-/tmp/gh-aw/cache-memory}"\n` +
148148
`${ri}# Strip execute bits from all non-.git files to prevent execute-bit\n` +
149149
`${ri}# persistence of attacker-planted executables across cache restore cycles.\n` +
150-
`${ri}find "$CACHE_DIR" -not -path '*/.git/*' -type f -exec chmod a-x {} +\n` +
151-
`${ri}echo "Execute bits stripped from cache-memory working tree"\n`
150+
`${ri}if [ -d "$CACHE_DIR" ]; then\n` +
151+
`${ri} find "$CACHE_DIR" -not -path '*/.git/*' -type f -exec chmod a-x {} + || true\n` +
152+
`${ri} echo "Execute bits stripped from cache-memory working tree"\n` +
153+
`${ri}else\n` +
154+
`${ri} echo "Skipping execute-bit stripping; cache-memory directory not present"\n` +
155+
`${ri}fi\n`
152156
);
153157
}
154158

@@ -177,11 +181,11 @@ function buildScanInjectionStep(indent: string): string {
177181
`${ri} mkdir -p "$QUARANTINE_DIR"\n` +
178182
`${ri} for f in "\${SUSPICIOUS_FILES[@]}"; do\n` +
179183
`${ri} rel="\${f#\${CACHE_DIR}/}"\n` +
180-
`${ri} dest="$QUARANTINE_DIR/\${rel//\\//_}"\n` +
181184
`${ri} echo "::warning::Quarantining file with instruction-shaped content: $f"\n` +
182185
`${ri} echo "--- First 5 lines of quarantined file: $f ---"\n` +
183-
`${ri} head -5 "$f" || true\n` +
184-
`${ri} mv -f "$f" "$dest"\n` +
186+
`${ri} head -5 "$f" | sed 's/^/| /' || true\n` +
187+
`${ri} mkdir -p "$QUARANTINE_DIR/$(dirname "$rel")"\n` +
188+
`${ri} mv -f "$f" "$QUARANTINE_DIR/$rel"\n` +
185189
`${ri} done\n` +
186190
`${ri} echo "Quarantined \${#SUSPICIOUS_FILES[@]} file(s) with instruction-shaped content to $QUARANTINE_DIR"\n` +
187191
`${ri}else\n` +
@@ -559,24 +563,40 @@ for (const workflowPath of workflowPaths) {
559563
}
560564

561565
// Update the main cache key lines and restore-keys prefix to include the date
562-
// for a 1-day TTL. Idempotent: only transforms when CACHE_MEMORY_DATE is not
563-
// yet present in the file.
564-
if (content.includes(cacheDateStepSentinel) && !content.includes(cacheDateRestoreKeySentinel)) {
565-
const before = content;
566+
// for a 1-day TTL. Apply each transformation independently so partially
567+
// updated workflows are repaired correctly and repeated runs stay idempotent.
568+
if (content.includes(cacheDateStepSentinel)) {
569+
let updatedCacheKey = false;
570+
let updatedRestoreKeys = false;
571+
566572
// Update main key: insert date before run_id
573+
const beforeKeyUpdate = content;
567574
content = content.replace(
568575
cacheMemoryKeyLineRegex,
569576
(_m, prefix) => `${prefix}\${{ env.CACHE_MEMORY_DATE }}-\${{ github.run_id }}`
570577
);
578+
updatedCacheKey = content !== beforeKeyUpdate;
579+
571580
// Update restore-keys prefix: append date segment
581+
const beforeRestoreKeysUpdate = content;
572582
content = content.replace(
573583
cacheRestoreKeyPrefixRegex,
574584
(_m, prefixWithWorkflowId, newline) =>
575585
`${prefixWithWorkflowId}\${{ env.CACHE_MEMORY_DATE }}-${newline}`
576586
);
577-
if (content !== before) {
587+
updatedRestoreKeys = content !== beforeRestoreKeysUpdate;
588+
589+
if (updatedCacheKey || updatedRestoreKeys) {
578590
modified = true;
579-
console.log(` Updated cache key and restore-keys to include CACHE_MEMORY_DATE for 1-day TTL`);
591+
if (updatedCacheKey && updatedRestoreKeys) {
592+
console.log(` Updated cache key and restore-keys to include CACHE_MEMORY_DATE for 1-day TTL`);
593+
} else if (updatedCacheKey) {
594+
console.log(` Updated cache key to include CACHE_MEMORY_DATE for 1-day TTL`);
595+
} else {
596+
console.log(` Updated restore-keys to include CACHE_MEMORY_DATE for 1-day TTL`);
597+
}
598+
} else {
599+
console.log(` Cache key/restore-keys already include CACHE_MEMORY_DATE`);
580600
}
581601
} else if (content.includes(cacheDateRestoreKeySentinel)) {
582602
console.log(` Cache key/restore-keys already include CACHE_MEMORY_DATE`);

0 commit comments

Comments
 (0)