Skip to content

fix(post-code): block workflow file changes and correct token docs - #2656

Closed
ascerra wants to merge 1 commit into
mainfrom
fix/code-agent-security-hardening
Closed

fix(post-code): block workflow file changes and correct token docs#2656
ascerra wants to merge 1 commit into
mainfrom
fix/code-agent-security-hardening

Conversation

@ascerra

@ascerra ascerra commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Corrects misleading code-agent.env comment that claimed the sandbox GH_TOKEN was read-only (contents:read, issues:read, pull_requests:read). The mint service actually grants contents:write, issues:write, pull_requests:write, checks:read — the same token used as PUSH_TOKEN on the runner. Adds a TODO for minting a separate read-only sandbox token.

  • Adds .github/workflows/ block in post-code.sh as defense-in-depth. The coder token already omits workflows:write (GitHub rejects the push server-side), but an explicit script-level gate ensures protection survives future role changes or GitHub permission model shifts. Addresses the prompt injection attack vector where a malicious issue comment could cause the code agent to create a workflow file that executes arbitrary code on the org's runner with access to repo secrets.

  • Adds 6 unit tests for the workflow file detection logic in post-code-test.sh.

Context

Discussion from today's fullsend team sync on code agent security (Barak's concern about elevated privileges + prompt injection). See meeting notes and Slack thread for full context.

Test plan

  • All 58 post-code tests pass (including 6 new workflow detection tests)
  • shellcheck clean on changed files (only pre-existing SC2001 style warnings)
  • Security self-review via code-review skill — approved with one low finding (GHA command injection in ::error:: output) which was fixed before push
  • CI passes

Made with Cursor

@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@ascerra
ascerra marked this pull request as draft June 25, 2026 13:54
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Harden post-code: block workflow changes and fix sandbox token docs
🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

Description

• Correct sandbox GH_TOKEN scope documentation; note it is write-capable and shared with runner.
• Block any agent commits touching .github/workflows/ before pushing to GitHub.
• Add unit tests covering workflow-path detection and non-workflow pass cases.
Diagram

graph TD
  A["Code agent commit"] --> B["post-code.sh"] --> C{"
  workflow files?
  "}
  C -->|"yes"| D["Fail: block push"]
  C -->|"no"| E["Secret scan"] --> F["Git push"] --> G["GitHub repo"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Rely only on GitHub token permissions (no script gate)
  • ➕ No additional maintenance/testing of path-detection logic
  • ➕ Server-side enforcement is authoritative
  • ➖ Brittle against future role/permission changes
  • ➖ Harder to reason about security posture during incidents
  • ➖ Still allows prompt-injected attempts to reach the push step
2. Enforce protected paths via GitHub-side controls (branch protection / rulesets)
  • ➕ Centralized and authoritative control at the repo/org level
  • ➕ Applies uniformly across tooling beyond this pipeline
  • ➖ May not be available/granular enough for all repos/branches
  • ➖ Can be more complex to roll out consistently
  • ➖ Doesn’t provide early failure/clearer agent feedback in this script
3. Generalize to a configurable protected-path denylist in post-code.sh
  • ➕ Scales beyond workflows to other high-risk paths without new code
  • ➕ Easier policy updates via config
  • ➖ More policy surface area to design and test
  • ➖ Risk of overblocking legitimate agent changes if misconfigured

Recommendation: Keep the script-level workflow block as the primary defense-in-depth control, since it fails early and remains effective even if GitHub permissions drift. Consider a follow-up to centralize protected-path policy (e.g., configurable denylist) to avoid one-off checks as new sensitive paths are identified.

Files changed (3) +127 / -12

Bug fix (1) +37 / -7
post-code.shBlock .github/workflows changes before push +37/-7

Block .github/workflows changes before push

• Adds a pre-push gate that detects any changed files under .github/workflows/ and exits with annotated errors. Documents the threat model (prompt-injected workflow creation) and positions this as primary enforcement beyond GitHub’s current token permission model.

internal/scaffold/fullsend-repo/scripts/post-code.sh

Tests (1) +79 / -0
post-code-test.shAdd workflow-path detection unit tests +79/-0

Add workflow-path detection unit tests

• Introduces a helper that mirrors the workflow-file detection logic and adds six tests covering workflow changes, mixed file sets, non-workflow .github paths, empty input, and nested workflows.

internal/scaffold/fullsend-repo/scripts/post-code-test.sh

Documentation (1) +11 / -5
code-agent.envFix GH_TOKEN scope comment; document workflow permission rationale +11/-5

Fix GH_TOKEN scope comment; document workflow permission rationale

• Updates documentation to reflect that the sandbox GH_TOKEN is write-capable and matches the runner’s push token. Clarifies that workflow pushes are rejected due to missing workflows:write and adds a TODO to mint a truly read-only sandbox token.

internal/scaffold/fullsend-repo/env/code-agent.env

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

Site preview

Preview: https://31969fd4-site.fullsend-ai.workers.dev

Commit: 4fa4034ff20d71a0dc2321eac5c139dd33775611

The code-agent.env comment falsely claimed the sandbox GH_TOKEN was
read-only (contents:read, issues:read, pull_requests:read). In reality
the mint service grants the coder role contents:write, issues:write,
pull_requests:write, checks:read — the same token used as PUSH_TOKEN
on the runner. Correct the comment to reflect the actual token scope
and add a TODO for minting a separate read-only sandbox token.

Add a defense-in-depth block in post-code.sh that rejects any agent
commit touching .github/workflows/ before push. The coder token already
omits workflows:write so GitHub would reject the push server-side, but
an explicit script-level gate ensures the protection survives future
role changes or GitHub permission model shifts. This addresses the
prompt injection attack vector where a malicious issue comment could
cause the code agent to create a workflow file that executes arbitrary
code on the org's runner with access to repo secrets.

Closes #2654

Signed-off-by: Adam Scerra <ascerra@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:59 PM UTC · Completed 2:12 PM UTC
Commit: 4fa4034 · View workflow run →

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 58 rules

Grey Divider


Remediation recommended

1. Workflow tests duplicate logic 🐞 Bug ⚙ Maintainability
Description
post-code-test.sh reimplements the workflow-file blocking matcher instead of calling a shared
implementation, so the tests can still pass if post-code.sh’s real enforcement drifts or is
accidentally changed/removed. This reduces confidence in a security-critical guardrail meant to
prevent workflow-file pushes.
Code

internal/scaffold/fullsend-repo/scripts/post-code-test.sh[R697-718]

+# ---------------------------------------------------------------------------
+# Test helper — reimplements the workflow file detection logic from
+# post-code.sh section 2c. Given a list of changed files, returns whether
+# the push would be blocked due to .github/workflows/ changes.
+# ---------------------------------------------------------------------------
+detect_workflow_files() {
+  local changed_files="$1"
+  local workflow_files=""
+
+  while IFS= read -r file; do
+    [ -z "${file}" ] && continue
+    case "${file}" in
+      .github/workflows/*) workflow_files="${workflow_files}${file}"$'\n' ;;
+    esac
+  done <<< "${changed_files}"
+
+  if [ -n "${workflow_files}" ]; then
+    echo "blocked:workflow"
+  else
+    echo "pass"
+  fi
+}
Relevance

⭐⭐⭐ High

Repo accepted requests for integration tests to avoid mirrored logic drift; likely accept
shared-impl change.

PR-#2341

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test file explicitly states it reimplements the production logic, meaning it is not asserting
behavior of the enforcement code path in post-code.sh.

internal/scaffold/fullsend-repo/scripts/post-code-test.sh[697-718]
internal/scaffold/fullsend-repo/scripts/post-code.sh[227-251]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`post-code-test.sh` duplicates the workflow-file detection logic rather than exercising the production implementation in `post-code.sh`. This creates a regression risk: changes to the real gate (or accidental removal) may not be caught by tests.

### Issue Context
- The production enforcement is implemented inline in `post-code.sh` (workflow-file block).
- The tests added a helper that *reimplements* that logic.

### Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-code.sh[227-251]
- internal/scaffold/fullsend-repo/scripts/post-code-test.sh[697-775]

### Suggested fix
1. Extract workflow-file detection into a small, side-effect-free shared function file (e.g. `internal/scaffold/fullsend-repo/scripts/lib/post-code-guards.sh`) that exports something like `detect_workflow_files_from_list()`.
2. In `post-code.sh`, `source` that file and call the shared function on `${CHANGED_FILES}`.
3. In `post-code-test.sh`, `source` the same file and test the shared function directly (keep the same test cases).
4. Add a minimal smoke test asserting the script exits non-zero when the shared function reports workflow files (optional but strengthens coupling).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Workflow block lacks requirement 📘 Rule violation § Compliance
Description
New push-blocking behavior for .github/workflows/ changes is introduced without any explicit
tracked requirement reference (ticket/RFC/issue) in the code/comments near the change. This makes
the new behavior harder to audit back to an approved requirement.
Code

internal/scaffold/fullsend-repo/scripts/post-code.sh[R227-251]

+# ---------------------------------------------------------------------------
+# 2c. Block workflow file changes (defense-in-depth)
+#
+# The coder token intentionally omits workflows:write, so GitHub should
+# reject pushes that modify .github/workflows/. This check is a
+# defense-in-depth measure: if the coder role ever gains workflows:write
+# (or GitHub changes its permission model), this gate prevents a
+# prompt-injected agent from pushing malicious workflow files that
+# execute on PR creation with access to repo secrets.
+# ---------------------------------------------------------------------------
+WORKFLOW_FILES=""
+while IFS= read -r file; do
+  [ -z "${file}" ] && continue
+  case "${file}" in
+    .github/workflows/*) WORKFLOW_FILES="${WORKFLOW_FILES}${file}"$'\n' ;;
+  esac
+done <<< "${CHANGED_FILES}"
+
+if [ -n "${WORKFLOW_FILES}" ]; then
+  echo "::error::BLOCKED — agent committed changes to .github/workflows/" >&2
+  echo "::error::Workflow file modifications require human authorship." >&2
+  echo "::error::Files blocked:" >&2
+  echo "${WORKFLOW_FILES}" | sed '/^$/d' | sed 's/::/%3A%3A/g; s/^/  /' >&2
+  exit 1
+fi
Relevance

⭐⭐ Medium

No historical evidence this repo enforces explicit ticket/RFC requirement references for new
behavior.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The cited code adds a new, user-visible enforcement gate (blocking pushes if workflow files are
changed). There is no explicit requirement/ticket reference in or near this added logic, which
violates the requirement-traceability rule for new behaviors.

Rule 1062064: New code must map to explicit tracked requirements; no speculative features
internal/scaffold/fullsend-repo/scripts/post-code.sh[227-251]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new enforcement behavior was added (blocking pushes that modify `.github/workflows/`). The compliance rule requires new behaviors to be traceable to an explicit tracked requirement (ticket/RFC/change request).

## Issue Context
Add a short reference near the new logic (e.g., `SEC-1234` / GitHub issue link) so auditors/reviewers can trace why this behavior exists and who approved it.

## Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-code.sh[227-251]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. code-agent.env comment-only change 📘 Rule violation ⚙ Maintainability
Description
The hunk updates only comments in code-agent.env without changing any adjacent code. This violates
the rule requiring comment edits to be coupled with a related code change in the same hunk.
Code

internal/scaffold/fullsend-repo/env/code-agent.env[R4-14]

+# GH_TOKEN in the sandbox is a WRITE-CAPABLE app installation token
+# (contents:write, issues:write, pull_requests:write, checks:read).
+# Set by setup-agent-env.sh from CODE_GH_TOKEN. This is the same token
+# used as PUSH_TOKEN on the runner — isolation is behavioral
+# (disallowedTools blocks git push / gh api) and network-level (L7
+# policy restricts GitHub API access to gh/git binaries), NOT
+# token-scoped. The coder role intentionally omits workflows:write,
+# so GitHub rejects pushes that modify .github/workflows/ files.
+#
+# TODO: mint a separate read-only token for sandbox use so that
+# disallowedTools bypass cannot escalate to write operations.
Relevance

⭐ Low

Comment-only edits commonly accepted in scaffold env/docs; no evidence of strict coupling
enforcement.

PR-#286
PR-#2395

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist disallows comment-only hunks. The cited region shows only # comment lines being
changed/added without any accompanying code-line modification in that hunk.

Rule 1062072: Do not add or modify comments outside code lines changed for the issue
internal/scaffold/fullsend-repo/env/code-agent.env[4-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`internal/scaffold/fullsend-repo/env/code-agent.env` contains a diff hunk that changes only comments. The compliance rule requires comment changes to be coupled with a related code change in the same hunk, or otherwise avoided.

## Issue Context
This PR updates token-scope documentation in comments only. To comply, either (a) revert/move these comment edits to a more appropriate location, or (b) pair the comment update with a meaningful, related code change in the same hunk.

## Fix Focus Areas
- internal/scaffold/fullsend-repo/env/code-agent.env[4-14]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. post-code.sh comment-only edits 📘 Rule violation ⚙ Maintainability
Description
The header section changes only comments describing security layers/protected-path behavior without
changing any code in that hunk. This violates the rule against comment-only hunks not paired with a
related code change.
Code

internal/scaffold/fullsend-repo/scripts/post-code.sh[R8-23]

# Security layers (defense-in-depth):
-#   1. Authoritative secret scan — final gate before any push
-#   2. Authoritative pre-commit — run repo hooks on changed files
-#   3. Branch validation — refuse to push main/master
-#   4. Token isolation — PUSH_TOKEN never enters the sandbox
+#   1. Workflow file block — reject .github/workflows/ changes
+#   2. Authoritative secret scan — final gate before any push
+#   3. Authoritative pre-commit — run repo hooks on changed files
+#   4. Branch validation — refuse to push main/master
+#   5. Token scope — coder token omits workflows:write (GitHub rejects
+#      workflow pushes server-side, but the script-level block is the
+#      primary enforcement so we don't depend on GitHub's permission model)
#
# Pre-commit tool deps are auto-installed from .pre-commit-tools.yaml
# before step 2 to ensure hooks have the binaries they need.
#
-# Protected-path enforcement lives in post-review.sh: the review agent
-# cannot approve PRs that touch sensitive paths (e.g. .github/, CODEOWNERS,
-# agents/). The code agent is free to propose changes to any path.
+# Protected-path enforcement for review approval lives in post-review.sh:
+# the review agent cannot approve PRs that touch sensitive paths (e.g.
+# .github/, CODEOWNERS, agents/). The code agent may propose changes to
+# non-workflow paths; human approval is required for those PRs.
Relevance

⭐ Low

Comment-only header edits in post-scripts have been accepted historically; no coupling rule
enforcement seen.

PR-#286
PR-#2395

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The compliance rule flags hunks that only change comments. The referenced lines are purely comment
updates in the script header (no code modifications in that region/hunk).

Rule 1062072: Do not add or modify comments outside code lines changed for the issue
internal/scaffold/fullsend-repo/scripts/post-code.sh[8-23]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`internal/scaffold/fullsend-repo/scripts/post-code.sh` has a diff hunk where only comments were edited (no code changes in that hunk). The compliance rule requires comment edits to be coupled with related code changes in the same hunk.

## Issue Context
This PR added real enforcement logic later in the file, but the header comment edits appear as a comment-only hunk.

## Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/post-code.sh[8-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

High

  • [secret-exposure] internal/scaffold/fullsend-repo/env/code-agent.env:9 — The sandbox receives a GH_TOKEN with contents:write, issues:write, pull_requests:write, and checks:read permissions (the coder role token). This is the same token used as PUSH_TOKEN on the runner (confirmed via internal/mintcore/github.go canonicalRolePermissions and internal/cli/run.go roleTokenVars). The sandbox agent can use this token to push code directly, bypassing post-code.sh gates. The PR body acknowledges this and mentions a TODO for minting a separate read-only sandbox token, but that TODO is not present in the diff — the old safety claim was removed without adding the actual safety measure or a code-level TODO.
    Remediation: Mint a separate read-only token for the sandbox (contents:read, issues:read, pull_requests:read). The coder token with write permissions should only be available to the runner post-script.

Medium

  • [missing-authorization] This non-trivial PR lacks a linked GitHub issue. The PR makes security-impacting changes (corrects token scope documentation, adds workflow file blocking logic, adds 6 new unit tests) without a linked issue. The PR body references a team sync discussion but provides no traceable artifact.
    Remediation: File a GitHub issue documenting the security discussion and link it to this PR.

  • [logic error — incomplete defense-in-depth] internal/scaffold/fullsend-repo/scripts/post-fix.sh — post-fix.sh (the fix agent's post-script) uses the same coder role token (confirmed: harness/fix.yaml declares role: coder) but does not include the new .github/workflows/ block added to post-code.sh. The fix agent can push workflow file changes without being stopped by the script-level gate.
    Remediation: Add the same .github/workflows/ detection-and-block logic to post-fix.sh, or extract it into a shared function sourced by both post-scripts.


Labels: PR adds security hardening to sandbox post-scripts and corrects token scope documentation

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

# rejects pushes that modify .github/workflows/ files.
export GH_TOKEN=${GH_TOKEN}

# Git identity — uses the GitHub App bot user's noreply email so GitHub

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] secret-exposure

The sandbox receives a GH_TOKEN with contents:write, issues:write, pull_requests:write, and checks:read permissions (the coder role token). This is the same token used as PUSH_TOKEN on the runner (confirmed via internal/mintcore/github.go canonicalRolePermissions and internal/cli/run.go roleTokenVars). The sandbox agent can use this token to push code directly, bypassing post-code.sh gates. The PR body acknowledges this and mentions a TODO for minting a separate read-only sandbox token, but that TODO is not present in the diff.

Suggested fix: Mint a separate read-only token for the sandbox (contents:read, issues:read, pull_requests:read). The coder token with write permissions should only be available to the runner post-script.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not in-scope for this PR! We do have an issue: #2826

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but what about the fix agent?

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an automated comment. We are moving agent content from internal/scaffold/fullsend-repo/ to https://github.com/fullsend-ai/agents -- changes should be made to agent definitions there going forwards.

@ifireball ifireball left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but not sure the is the behaviour we want.

See: #2908 and #2822

@ascerra

ascerra commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@ifireball ack I created this quickly as we were discussing internally how to improve topics around agent privilege.

I believe this is more of a workaround for what I believe is the most important thing to solve which is #2826 from your planned work.

I will close this as I believe that is the better direction for us to be focussed on.

@ascerra ascerra closed this Jul 8, 2026
@rh-hemartin

Copy link
Copy Markdown
Member

Remember to delete the branch if you don't need it anymore.

@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:13 PM UTC · Completed 3:21 PM UTC
Commit: 4fa4034 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

PR #2656 was a human-authored security hardening PR by ascerra that corrected misleading token scope documentation in code-agent.env, added .github/workflows/ blocking in post-code.sh, and added 6 unit tests. It was open for 13 days as a draft, received reviews from 4 humans and 2 bots, and was ultimately closed without merge when the author recognized it was a workaround for what should be solved architecturally via issue #2826.

The review agent performed well in some areas: it correctly identified that post-fix.sh lacked the same workflow-blocking logic (independently confirmed by human reviewer rh-hemartin), and accurately traced actual token permissions through mintcore code. However, it flagged the pre-existing write-capable sandbox token as a high-severity finding despite the PR body explicitly acknowledging this gap — a human reviewer had to comment 'not in-scope for this PR!' The review agent also missed the most consequential concern: ralphbean flagged 11 days later that the modified files were actively being migrated to the fullsend-ai/agents repo, which was a key factor in the PR's closure.

Two proposals target these review quality gaps.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/sandbox OpenShell sandbox environment security Security threat model and related concerns

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants