Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d46a2d61-1cc8-404f-a706-606b9f7e42c9 Co-authored-by: szabta89 <1330202+szabta89@users.noreply.github.com>
…d executables and disallowed files Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d46a2d61-1cc8-404f-a706-606b9f7e42c9 Co-authored-by: szabta89 <1330202+szabta89@users.noreply.github.com>
|
Hey The PR looks well-structured: the shell script hardening, the Go env-var wiring in This looks ready for maintainer review once the draft status is lifted. Nice one! 🎉
|
|
@copilot merge main and recompile |
…ory-issue Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6cdf49f7-b40e-4d7a-876c-231165f06b79 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 64f9d20 — merged main and recompiled all 191 workflow lock files successfully. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| if [ -n "${GH_AW_ALLOWED_EXTENSIONS:-}" ]; then | ||
| echo "Pre-agent sanitization: enforcing allowed extensions: ${GH_AW_ALLOWED_EXTENSIONS}" | ||
| removed=0 | ||
| while IFS= read -r file; do |
There was a problem hiding this comment.
Added setup_cache_memory_git_test.sh in fa194e9. It covers 8 test scenarios (23 assertions): execute-bit stripping, .git/ dir exclusion, no filter when GH_AW_ALLOWED_EXTENSIONS is unset, extension-based removal, files without extensions, single-extension allowlists, combined stripping+filtering, and case-sensitive extension comparison. All pass.
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification Details
Test Inflation Note
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR hardens cache-memory restores by adding a pre-agent sanitization gate in the git-setup script, and wiring allowed-extensions through to that setup step so unexpected file types from prior runs can be removed before the agent sees them.
Changes:
- Add pre-agent working-tree sanitization to
setup_cache_memory_git.sh(strip execute bits; optionally remove disallowed extensions). - Emit
GH_AW_ALLOWED_EXTENSIONSfromgenerateCacheMemoryGitSetupStepwhen extensions are configured. - Add unit tests asserting the env var is emitted/omitted as expected, and update a lock workflow snapshot accordingly.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/sh/setup_cache_memory_git.sh | Adds pre-agent sanitization (chmod a-x; optional extension-based deletion). |
| pkg/workflow/cache.go | Passes GH_AW_ALLOWED_EXTENSIONS into the setup step when configured. |
| pkg/workflow/cache_integrity_test.go | Adds tests for env var emission/omission in generated steps. |
| pkg/testutil/spec_test.go | Formatting-only adjustments in a test case struct literal. |
| .github/workflows/ai-moderator.lock.yml | Updates generated workflow snapshot to include the new env var. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/5 changed files
- Comments generated: 4
| # 1. Strip execute bits from all working-tree files so that a prior run cannot plant | ||
| # executable scripts (e.g. helper.sh) that the agent or runner could invoke before | ||
| # any validation gate fires. | ||
| find . -not -path './.git/*' -type f -exec chmod a-x {} + 2>/dev/null || true | ||
| echo "Pre-agent sanitization: stripped execute permissions from all working-tree files" | ||
|
|
||
| # 2. If GH_AW_ALLOWED_EXTENSIONS is set (colon-separated, e.g. .json:.md:.txt), remove |
There was a problem hiding this comment.
The execute-bit stripping uses find ... -type f, which skips symlinks. A prior run could plant a symlink (e.g., secrets.json -> /home/runner/.ssh/...) that the agent can read before threat detection, bypassing both execute-bit stripping and extension enforcement. Consider deleting symlinks in the cache working tree (e.g., find ... -type l -delete) or otherwise handling them explicitly as part of pre-agent sanitization.
| # 1. Strip execute bits from all working-tree files so that a prior run cannot plant | |
| # executable scripts (e.g. helper.sh) that the agent or runner could invoke before | |
| # any validation gate fires. | |
| find . -not -path './.git/*' -type f -exec chmod a-x {} + 2>/dev/null || true | |
| echo "Pre-agent sanitization: stripped execute permissions from all working-tree files" | |
| # 2. If GH_AW_ALLOWED_EXTENSIONS is set (colon-separated, e.g. .json:.md:.txt), remove | |
| # 1. Delete all working-tree symlinks so that a prior run cannot plant links to files | |
| # outside the cache (e.g. secrets) that would bypass the regular-file checks below. | |
| find . -not -path './.git/*' -type l -delete 2>/dev/null || true | |
| echo "Pre-agent sanitization: deleted all working-tree symlinks" | |
| # 2. Strip execute bits from all working-tree files so that a prior run cannot plant | |
| # executable scripts (e.g. helper.sh) that the agent or runner could invoke before | |
| # any validation gate fires. | |
| find . -not -path './.git/*' -type f -exec chmod a-x {} + 2>/dev/null || true | |
| echo "Pre-agent sanitization: stripped execute permissions from all working-tree files" | |
| # 3. If GH_AW_ALLOWED_EXTENSIONS is set (colon-separated, e.g. .json:.md:.txt), remove |
| while IFS= read -r file; do | ||
| filename="$(basename "$file")" | ||
| # Extract the last dot-prefixed segment as the extension, or empty if no dot | ||
| case "$filename" in | ||
| *.*) ext=".${filename##*.}" ;; | ||
| *) ext="" ;; | ||
| esac | ||
| # Check whether this extension appears in the colon-separated allowed list | ||
| found=0 | ||
| IFS=: read -ra _ALLOWED_EXTS <<< "$GH_AW_ALLOWED_EXTENSIONS" | ||
| for _a in "${_ALLOWED_EXTS[@]}"; do | ||
| if [ "$ext" = "$_a" ]; then | ||
| found=1 | ||
| break | ||
| fi | ||
| done | ||
| if [ "$found" -eq 0 ]; then | ||
| echo "Removing disallowed file: $file (extension: '${ext:-none}')" | ||
| rm -f "$file" | ||
| removed=$((removed + 1)) | ||
| fi | ||
| done < <(find . -not -path './.git/*' -type f) |
There was a problem hiding this comment.
The allowed-extension enforcement loop relies on newline-delimited find output and read -r file, which breaks for filenames containing newlines and makes it possible for a crafted filename to evade checks/removal. Consider using NUL delimiters (find ... -print0 with while IFS= read -r -d '' file) for robust sanitization.
| # Extract the last dot-prefixed segment as the extension, or empty if no dot | ||
| case "$filename" in | ||
| *.*) ext=".${filename##*.}" ;; | ||
| *) ext="" ;; | ||
| esac | ||
| # Check whether this extension appears in the colon-separated allowed list | ||
| found=0 | ||
| IFS=: read -ra _ALLOWED_EXTS <<< "$GH_AW_ALLOWED_EXTENSIONS" | ||
| for _a in "${_ALLOWED_EXTS[@]}"; do | ||
| if [ "$ext" = "$_a" ]; then | ||
| found=1 | ||
| break | ||
| fi | ||
| done |
There was a problem hiding this comment.
Extension matching is currently case-sensitive and does not trim whitespace in GH_AW_ALLOWED_EXTENSIONS entries. This can cause unexpected deletions (e.g., configured ".MD" won’t match "readme.md", or ".json " won’t match ".json"). Consider normalizing both the extracted ext and the allowed list (trim + lowercase) once outside the per-file loop before comparisons.
| if len(cache.AllowedExtensions) > 0 { | ||
| fmt.Fprintf(builder, " GH_AW_ALLOWED_EXTENSIONS: '%s'\n", strings.Join(cache.AllowedExtensions, ":")) | ||
| } |
There was a problem hiding this comment.
GH_AW_ALLOWED_EXTENSIONS is inserted into the generated workflow YAML as a single-quoted string, but the value is built from user-configured extensions without YAML escaping/validation. If an extension contains a single quote, newline, or other YAML-significant characters, it can break the workflow (and potentially enable YAML injection in generated steps). Consider validating allowed-extensions entries to a strict pattern (e.g., ^\.[A-Za-z0-9]+$) and/or escaping for YAML single-quoted scalars (replace ' with '') before writing.
Generated by Design Decision Gate workflow to document the architectural decision to strip execute bits and filter disallowed extensions from the cache-memory working tree before agent execution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (>100 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft captures the key design decision: extend What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24514620900 Note 🔒 Integrity filter blocked 1 itemThe following item were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/25983d1f-b4ed-4392-b6f0-f641e5d31519 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copiolot review all comments |
…, NUL delimiters, extension normalization, YAML injection) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/634f95c6-4b33-413e-b69c-63111abced23 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All four review findings addressed in d3f511a:
Tests updated accordingly (shell tests 8–11 + new |
With
integrity: none(the default), cache-memory restores prior-run files into/tmp/gh-aw/cache-memory/before the agent runs and before threat detection fires. A compromised run can plant executable scripts (e.g.helper.sh), symlinks to sensitive files, or unexpected file types that the agent encounters without any prior validation gate.Changes
setup_cache_memory_git.sh— added a pre-agent sanitization block that runs aftergit checkout/merge-down, immediately before the agent can access the working tree:GH_AW_ALLOWED_EXTENSIONS: removes files whose extension isn't in the allowed list using NUL-delimitedfind -print0for safe handling of filenames with newlines; extension matching is case-insensitive and whitespace-tolerant (both the allowed list entries and extracted extensions are lowercased and whitespace-stripped before comparison)pkg/workflow/cache.go(generateCacheMemoryGitSetupStep) — passesGH_AW_ALLOWED_EXTENSIONSas a colon-separated env var to the git setup step whenAllowedExtensionsis configured. Extensions are validated at parse time against^\.[A-Za-z0-9]+$(viaisValidFileExtension) to prevent YAML injection, and single quotes are escaped ('→'') at emit time as defense-in-depth:pkg/workflow/cache_integrity_test.go— unit tests verifying the env var is emitted when extensions are configured, omitted when the list is empty, and thatisValidFileExtensioncorrectly accepts/rejects values and that single-quote escaping is applied in generated YAML.actions/setup/sh/setup_cache_memory_git_test.sh— shell tests for the sanitization block covering 11 scenarios (32 assertions):.git/directory is excluded from sanitizationGH_AW_ALLOWED_EXTENSIONSis unset.JSONfile matched by.jsonallowlist entry).jsonmatches.jsonfiles)The execute-bit stripping and symlink deletion are always active for all
cache-memoryworkflows. Extension-based removal only activates for workflows that explicitly setallowed-extensions.