Skip to content

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

Closed
guyoron1 wants to merge 534 commits into
mainfrom
mirror-2656-security-hardening
Closed

fix(post-code): block workflow file changes and correct token docs#82
guyoron1 wants to merge 534 commits into
mainfrom
mirror-2656-security-hardening

Conversation

@guyoron1

Copy link
Copy Markdown
Owner

Mirror of fullsend-ai#2656 for QualityFlow integration testing.

Original PR Summary

Security hardening: adds .github/workflows/ block in post-code.sh as defense-in-depth against prompt injection attacks where malicious issue comments could cause the code agent to create workflow files. Corrects misleading token permission docs. Includes 6 unit tests.

QualityFlow

This PR will be used to demonstrate QualityFlow's automated test planning and generation pipeline.

ggallen and others added 30 commits June 22, 2026 15:04
…ase4-pr3

refactor(config): remove legacy agent discovery fallbacks (ADR-0045 Phase 4 PR 3)
The merge queue uses merge (not squash), so individual commit messages
matter. Previously, commit-lint only checked the PR title on
pull_request events and deferred per-commit linting to merge_group.
This meant invalid prefixes (like `style`) only surfaced when the
merge queue rejected the PR — too late for contributors to fix easily.

Now commit-lint checks individual commits on pull_request events too,
giving early feedback before the merge queue.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Promote the missing-role check from a Lint() warning to a hard
Validate() error.  Every scaffold harness already sets role:, so this
is non-breaking for existing users while enforcing the contract
going forward.

- Validate() now returns "role field is required" when Role is empty
- Lint() no longer emits the role-is-not-set diagnostic
- Role struct tag keeps omitempty (Validate() is the enforcement)
- ADR-0045 struct example consistent with code
- Phase 4 plan updated to mark PR 1 as in-review
- Removed trivially-passing NoLintWarningWithRole tests
- All test fixtures updated to include role: where needed

Signed-off-by: Greg Allen <gallen@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
Promote the missing-role check from a Lint() warning to a hard
Validate() error.  Every scaffold harness already sets role:, so this
is non-breaking for existing users while enforcing the contract
going forward.

- Validate() now returns "role field is required" when Role is empty
- Lint() no longer emits the role-is-not-set diagnostic
- Role struct tag keeps omitempty (Validate() is the enforcement)
- ADR-0045 struct example consistent with code
- Phase 4 plan updated to mark PR 1 as in-review
- Removed trivially-passing NoLintWarningWithRole tests
- All test fixtures updated to include role: where needed

Signed-off-by: Greg Allen <gallen@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
…view

feat(harness): require role field in Validate()
…view

feat(harness): require role field in Validate()
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
…egrity

docs(security): add audit log integrity to threat model
…egrity

docs(security): add audit log integrity to threat model
Add a functional test framework for agent pipelines using
agent-eval-harness lifecycle hooks. The harness drives case iteration
with before_each/after_each hooks for ephemeral repo management,
while fullsend runs inside openshell sandboxes.

Key components:
- eval/scripts/setup-fixture.sh: before_each hook creates ephemeral
  GitHub repos and fixtures (issues/PRs) from input.yaml
- eval/scripts/run-fullsend.sh: CLI runner invokes fullsend run
- eval/scripts/capture-fixture.sh: after_each hook snapshots fixture
  state for judges
- eval/scripts/teardown-fixture.sh: after_each hook deletes repos
- eval/run-functional.sh: orchestrator calling workspace.py,
  execute.py, and score.py with behavioral threshold checks
- eval/triage/: first eval suite with LLM judge and label checks

Also includes CI workflow, behavioral thresholds (max_turns,
max_cost_usd), metrics capture from Claude Code stream events,
ADRs, and documentation.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Add a functional test framework for agent pipelines using
agent-eval-harness lifecycle hooks. The harness drives case iteration
with before_each/after_each hooks for ephemeral repo management,
while fullsend runs inside openshell sandboxes.

Key components:
- eval/scripts/setup-fixture.sh: before_each hook creates ephemeral
  GitHub repos and fixtures (issues/PRs) from input.yaml
- eval/scripts/run-fullsend.sh: CLI runner invokes fullsend run
- eval/scripts/capture-fixture.sh: after_each hook snapshots fixture
  state for judges
- eval/scripts/teardown-fixture.sh: after_each hook deletes repos
- eval/run-functional.sh: orchestrator calling workspace.py,
  execute.py, and score.py with behavioral threshold checks
- eval/triage/: first eval suite with LLM judge and label checks

Also includes CI workflow, behavioral thresholds (max_turns,
max_cost_usd), metrics capture from Claude Code stream events,
ADRs, and documentation.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
- Missing metrics.json is now a FAIL (not a warning), ensuring
  behavioral thresholds cannot be silently bypassed when the agent
  crashes or fullsend run fails.
- Validate that jq output is numeric before threshold comparison,
  preventing null/malformed values from silently passing as 0.
- Add shellcheck SC2317 disable directives for trap handler commands
  that shellcheck incorrectly flags as unreachable.

Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
- Missing metrics.json is now a FAIL (not a warning), ensuring
  behavioral thresholds cannot be silently bypassed when the agent
  crashes or fullsend run fails.
- Validate that jq output is numeric before threshold comparison,
  preventing null/malformed values from silently passing as 0.
- Add shellcheck SC2317 disable directives for trap handler commands
  that shellcheck incorrectly flags as unreachable.

Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The harness was referenced twice: once as a git submodule and once as a
pip install from the same git URL. Install from the already-checked-out
submodule so the fork URL only appears in .gitmodules.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The harness was referenced twice: once as a git submodule and once as a
pip install from the same git URL. Install from the already-checked-out
submodule so the fork URL only appears in .gitmodules.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Move max_turns and max_cost_usd checks from custom shell code in
run-functional.sh into deterministic check judges in eval.yaml. The
harness's score.py now enforces these via min_pass_rate: 1.0 thresholds.

Extract the pre-flight annotation validation into a standalone
eval/lint-cases.sh linter, wired up as `make lint-eval-cases` and
included in `make test`. This runs cheaply without executing agents.

Net effect: ~90 lines removed from run-functional.sh.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Move max_turns and max_cost_usd checks from custom shell code in
run-functional.sh into deterministic check judges in eval.yaml. The
harness's score.py now enforces these via min_pass_rate: 1.0 thresholds.

Extract the pre-flight annotation validation into a standalone
eval/lint-cases.sh linter, wired up as `make lint-eval-cases` and
included in `make test`. This runs cheaply without executing agents.

Net effect: ~90 lines removed from run-functional.sh.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Extend lint-cases.sh to verify that eval.yaml declares max_turns and
max_cost judges, not just that annotations.yaml declares the thresholds.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Extend lint-cases.sh to verify that eval.yaml declares max_turns and
max_cost judges, not just that annotations.yaml declares the thresholds.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The CLI runner receives {output_dir} which is workspace/output. Writing
metrics.json to $OUTPUT_DIR/output/ created a double-nested path that
score.py couldn't find. Write to $OUTPUT_DIR/metrics.json instead so
the file appears at the expected output/metrics.json key.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The CLI runner receives {output_dir} which is workspace/output. Writing
metrics.json to $OUTPUT_DIR/output/ created a double-nested path that
score.py couldn't find. Write to $OUTPUT_DIR/metrics.json instead so
the file appears at the expected output/metrics.json key.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The triage agent consistently takes ~23 turns on this case. The
previous threshold of 15 was too tight.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The triage agent consistently takes ~23 turns on this case. The
previous threshold of 15 was too tight.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Add three new triage eval cases covering the remaining major outcomes:

- 002-needs-info-vague-crash: vague issue with no repro steps, expects
  action "insufficient" and needs-info label
- 003-feature-request: clear feature request, expects action "sufficient"
  with category "feature" and triaged+feature labels (not ready-to-code)
- 004-duplicate-issue: issue duplicating a seed issue, expects action
  "duplicate" with duplicate label

Supporting changes:
- setup-fixture.sh: support seed_issues in input.yaml for pre-populating
  issues before the main fixture (needed by duplicate test)
- eval.yaml: add forbidden_labels judge to verify wrong labels are NOT
  applied (needs-info must not get ready-to-code, etc.)
- lint-cases.sh: check for forbidden_labels judge presence

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Add three new triage eval cases covering the remaining major outcomes:

- 002-needs-info-vague-crash: vague issue with no repro steps, expects
  action "insufficient" and needs-info label
- 003-feature-request: clear feature request, expects action "sufficient"
  with category "feature" and triaged+feature labels (not ready-to-code)
- 004-duplicate-issue: issue duplicating a seed issue, expects action
  "duplicate" with duplicate label

Supporting changes:
- setup-fixture.sh: support seed_issues in input.yaml for pre-populating
  issues before the main fixture (needed by duplicate test)
- eval.yaml: add forbidden_labels judge to verify wrong labels are NOT
  applied (needs-info must not get ready-to-code, etc.)
- lint-cases.sh: check for forbidden_labels judge presence

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Cases use isolated ephemeral repos with UUID suffixes, so there's no
shared state. Sequential execution took ~15 min of agent time across
4 cases; parallel should bring wall-clock down to ~6 min.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Cases use isolated ephemeral repos with UUID suffixes, so there's no
shared state. Sequential execution took ~15 min of agent time across
4 cases; parallel should bring wall-clock down to ~6 min.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
ADR 0045 was taken on main by forge-portable-harness-schema while this
branch was out of date. Renumber both branch ADRs and update all
cross-references.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
ADR 0045 was taken on main by forge-portable-harness-schema while this
branch was out of date. Renumber both branch ADRs and update all
cross-references.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
ralphbean and others added 27 commits June 24, 2026 10:31
resolveBaseScripts used urlDirPrefix to resolve script relative paths
against the YAML file's URL directory. But script paths in harness
YAMLs are relative to the scaffold root (the parent of harness/), not
the YAML file itself — matching local resolution where
ResolveRelativeTo is called with absFullsendDir (the workspace root).

Add urlParentDirPrefix that goes up one additional directory level and
use it in resolveBaseScripts. Fix existing tests that encoded the bug
by mounting scripts under /harness/scripts/ instead of /scripts/.

Fixes fullsend-ai#2610

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Remove --direct from TestAdminInstallUninstall so the e2e exercises the
new default PR-based delivery. Add mergeScaffoldPR helper (same pattern
as mergeEnrollmentPR) to find and merge the scaffold PR before verifying
files on the default branch. TestVendorFromSubdirectory keeps --direct
since it only tests GOMOD discovery.

Closes fullsend-ai#2558

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
Add a registry-based system for resolving and installing pre-commit
hook tool dependencies at runtime, replacing hardcoded tool installs
baked into OpenShell container images.

New files:
- tools/precommit-tools.yaml: registry mapping hook repos/IDs to
  system tools with pinned versions and SHA256 checksums
- scripts/resolve-precommit-tools.py: standalone Python resolver
  that parses .pre-commit-config.yaml against the registry
- scripts/resolve-precommit-tools.sh: bash wrapper ensuring PyYAML
  is available before invoking the Python resolver
- scripts/install-precommit-tools.sh: installs tools from the JSON
  manifest (binary/apt/pip/npm) with architecture detection

Modified pre/post scripts (pre-code, pre-fix, post-code, post-fix)
to call the resolver and installer instead of hardcoding tool
versions. Removes LYCHEE_VERSION/UV_VERSION constants from
post-code.sh and post-fix.sh.

Supply-chain hardening:
- Binary downloads use pinned versions + SHA256 checksums
- pip installs use --no-deps to prevent transitive dependency attacks
- npm installs use --ignore-scripts to prevent install-time RCE
- jq architecture lookups use --arg binding (not shell interpolation)
- PyYAML pinned to ==6.0.2
- Pre-scripts write to GITHUB_PATH for cross-step persistence

Closes fullsend-ai#1270

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
fix(install): skip enrollment in PR mode + e2e PR-based scaffold flow
…ipt-resolution

fix(harness): resolve URL base scripts relative to scaffold root
…eadiness-skill

feat: add autonomy-readiness skill for retro agent
…ize-test-speed

fix: disable CSMA spread delay in post-prioritize tests
When the CLI scaffolds workflow files during `fullsend github setup`,
pin the `uses:` directive and `fullsend_ai_ref` parameter to the
binary's build-time commit SHA instead of the mutable `@v0` tag.
Dev builds fall back to `@v0`.

Closes fullsend-ai#1933

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
…pr-commits

fix(ci): lint individual commits on PRs to catch invalid prefixes early
…mmit SHAs

PR fullsend-ai#2508 pinned actions in .github/workflows/ and
.github/actions/setup-gcp/ but missed the root action.yml composite
action. Repos with strict SHA-pinning policies (e.g. openkaiden/kaiden)
reject the unpinned tag refs, failing the Triage job with:

  "actions/setup-go@v6, actions/cache/restore@v4, and
   actions/upload-artifact@v7 are not allowed because all actions
   must be pinned to a full-length commit SHA"

Pin all five remaining tag refs to match the SHAs already used in the
workflow files:
- actions/setup-go@v6 → v6.4.0 SHA (×2)
- actions/cache/restore@v4 → v4.3.0 SHA
- actions/cache/save@v4 → v4.3.0 SHA
- actions/upload-artifact@v7 → v7.0.1 SHA

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
…affold-sha

feat(scaffold): pin workflow refs to release commit SHA
…ion-yml

fix(ci): pin third-party actions in root action.yml to commit SHAs
Replace hardcoded @v0 with __FULLSEND_AI_REF__ placeholder for the
mint-token composite action in prioritize-scheduler and
repo-maintenance scaffold templates. At scaffold time the placeholder
is replaced with a SHA pin, matching the pattern already used by
reusable workflow refs.

Without this, downstream .fullsend repos that enable
sha_pinning_required reject mint-token@v0 as an unpinned action ref.

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
… display

Remote skills loaded via URL were logged as "tree" during sandbox
bootstrap because filepath.Base() on the cache path returns the
last segment of GitHub's /tree/ URL convention. Add
resolveSkillDisplayName() that reads SKILL.md frontmatter via
skill.ParseFrontmatter() and uses the name field when available,
falling back to filepath.Base() for local skills or when no
frontmatter is present.

Note: pre-commit could not run in sandbox (shellcheck network
error). go vet and go test passed.

Closes fullsend-ai#2632
The pinact config and pre-commit hook only covered .github/workflows/,
.github/actions/, and the scaffold workflow directory. The root
action.yml was never scanned, which allowed unpinned tag refs to slip
through in PR fullsend-ai#2508 (fixed by fullsend-ai#2621).

Add action.yml to both .pinact.yaml file patterns and the pre-commit
hook's file regex so unpinned refs are caught automatically.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The repo contains .github/workflows/fullsend.yaml which was not matched
by the *.yml-only patterns. Add *.yaml patterns for both the root and
scaffold workflow directories so pinact scans all workflow files
regardless of extension.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…-display-name

fix(fullsend-ai#2632): use SKILL.md frontmatter name for remote skill display
…-root-action-yml

fix(ci): include root action.yml in pinact and pre-commit checks
Update scaffold_test.go assertions to expect
mint-token@__FULLSEND_AI_REF__ instead of mint-token@v0, matching
the template change.

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
…en-scaffold

fix(scaffold): use __FULLSEND_AI_REF__ for mint-token action refs
Scope automerge to patch bumps and pin updates via packageRules.
Combined with repo-level allow_auto_merge and the ruleset bypass
for renovate-fullsend, these low-risk updates merge automatically
after CI passes.

Closes fullsend-ai#2506

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
…te-automerge

feat(renovate): enable automerge for low-risk PRs
Add a dedicated macOS runner in lint.yml to exercise darwin-specific
sandbox behavior. Add TestUploadDir_SuppressesAppleDoubleInTarball on
darwin: verifies COPYFILE_DISABLE=1 prevents ._* files in tarballs using
python3 tarfile inspection, with a negative control to confirm xattr
application actually triggers AppleDouble generation without the flag.

Signed-off-by: Hector Martinez <hemartin@redhat.com>
…test

ci(sandbox): add macOS runner for darwin-specific tar behavior test
…ools

feat(scaffold): auto-detect and install pre-commit tool dependencies
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 fullsend-ai#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 4:01 PM UTC · Completed 4:03 PM UTC
Commit: db171e5 · View workflow run →

@guyoron1 guyoron1 closed this Jun 25, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already closed.

The /fs-review command only reviews open pull requests.

Posted by fullsend post-review check

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants