feat: harden smart-test-pipeline execution and validation - #1973
feat: harden smart-test-pipeline execution and validation#1973haseeb-heaven wants to merge 30 commits into
Conversation
…on, and failure exits
…nd review provenance
haseeb-heaven
changed the title
haseeb-heaven
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b61bc503a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e accounting, and failure reporting
…ets, and hardened Linux backend selection
…real sandbox detection
haseeb-heaven
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f760bad53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 938b4978e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52f2014abd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
haseeb-heaven
left a comment
haseeb-heaven
left a comment
There was a problem hiding this comment.
Additional Codex review feedback: one distinct host-side Git hook issue verified against the current head. Existing review threads were checked and this is not covered by them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88de634f41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c56e9e894
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e4babc2c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| agent_provider_env() { | ||
| # Provider secrets must never enter an agent-controlled subprocess. | ||
| printf '%s' "" | ||
| } |
There was a problem hiding this comment.
Captain, when the selected CLI authenticates through the documented *_PROVIDER_ENV API-key settings, this function always returns an empty allowlist, while the agent is also launched with env -i and a fresh empty home directory. None of the settings in config.example.sh can therefore reach the model process, so the agent fails authentication after review bots have already been triggered; honor the selected provider allowlist while continuing to exclude GitHub and unrelated credentials.
Useful? React with 👍 / 👎.
| docker run --rm --user "$(id -u):$(id -g)" "$network_arg" "${docker_env[@]}" \ | ||
| --read-only --tmpfs /tmp --mount "type=bind,src=$worktree,dst=$worktree" "${git_mount[@]}" \ | ||
| --mount "type=bind,src=$home_dir,dst=$home_dir" --mount "type=bind,src=$temp_dir,dst=$temp_dir" \ | ||
| -w "$worktree" "$image" bash -c 'exec "$@"' bash "${command[@]}" |
There was a problem hiding this comment.
When validation uses the advertised Docker backend, these mounts are never assigned to HOME or TMPDIR; the container commonly retains HOME=/root despite running as the host's non-root UID, and the root filesystem is read-only. Test and lint tools that create caches or configuration under home then fail even though writable per-stage directories were mounted, so pass HOME=$home_dir and TMPDIR=$temp_dir into the container.
Useful? React with 👍 / 👎.
|
|
||
| # The final review is collected after the last fix pass. A clean response | ||
| # here is a valid success, not an iteration-limit failure. | ||
| local final_dir="$DATA_DIR/iterations/$MAX_ITERATIONS" final_findings final_findings_file |
There was a problem hiding this comment.
When the final allowed fix pass succeeds, iterations/$MAX_ITERATIONS already contains that pass's findings and appended validation results. Reusing it here makes collect_findings overwrite findings.json, and the clean branch subsequently rewrites report.md, so the final report attributes the post-fix findings to the fix iteration and drops that iteration's test, lint, and CI results; store the terminal review separately or preserve the existing artifacts.
AGENTS.md reference: AGENTS.md:L37-L38
Useful? React with 👍 / 👎.
| elif jq -e '[.[] | select(.source == "github-ci" and (.path == "unknown" or .path == null))] | length > 0' "$findings_file" >/dev/null; then | ||
| allowed=true |
There was a problem hiding this comment.
When a pushed iteration has failing CI but no unresolved review threads, ci_findings creates a github-ci finding whose path is unknown; this branch then marks every non-forbidden changed path as allowed, regardless of its relationship to the failure. The networked fixer can consequently modify and have the orchestrator commit unrelated production code or even .github/workflows/*, defeating the advertised scope boundary; require explicit affected paths or approved support patterns, and stop for manual triage when CI cannot identify one.
Useful? React with 👍 / 👎.
| (allow file-read* (subpath "/usr") (subpath "/bin") (subpath "/sbin") (subpath "/System") (subpath "/Library")) | ||
| (allow file-read* (subpath "$worktree")) | ||
| (allow file-read* (subpath "$agent_home")) | ||
| (allow file-read* (subpath "$temp_dir")) | ||
| (allow file-read* (subpath "$executable")) |
There was a problem hiding this comment.
On the only provider-network-capable auto backend, a supported agent installed through Homebrew, npm, or another launcher commonly has an executable symlink under /opt/homebrew or /usr/local that loads an interpreter and package files elsewhere in that installation tree. This profile permits reading only the command path and system directories, so the CLI is denied its runtime files and exits before processing the brief even though preflight found it; resolve the launcher and expose only its required runtime/install tree read-only.
Useful? React with 👍 / 👎.
1 participant
Summary
Harden
smart-test-pipelineinto a production-ready, guarded PR review and fix loop. The pipeline now treats pull-request code, review text, CI output, and model-agent actions as separate trust boundaries and stops safely when a required control cannot be verified.Security and isolation
.gitcontrol data, refs, hooks, credentials, and the Git common directory.Correctness and reliability
BASH_SOURCE[0].Regression coverage
The focused suite covers malicious fork content, prompt-injected review comments, Git hook attacks, staged/untracked changes, agent-created commits, CI-only failures, failed commit/push/review waits, concurrent runs, config precedence, adapter invocation, dry-run behavior, and credential-free sandbox execution.
Validation
The no-mistakes review, focused tests, and documentation checks passed. Local ShellCheck parity was unavailable because ShellCheck 0.11.0 is not installed; this remains an environment warning for repository CI. No merge automation is included.