Skip to content

Factory PR-feedback commits are pushed without DCO sign-off #9186

Open

Description

@mytmlt

Problem

Commits made by the factory agent during Address PR feedback (and Fix pull request checks) can be pushed without a Signed-off-by trailer, which blocks the PR on the required DCO check.

Seen on:

Root cause

  • Implement checkouts run FactoryRepoCommitSetup() (pkg/components/runner/factory_commit_identity.go:98-110, called from workspace_agent_resources.go:686-703), which installs .git/hooks/prepare-commit-msg in the repo. Every git call runs that hook.
  • PR-feedback and fix-checks tasks are built with OmitCommitIdentity: true (create_factory_pr_feedback_handler.go:139, factory_app_templates.go:1005), which skips that setup (pr_feedback_template.go:659, pr_feedback_checks_template.go:248).
  • That leaves sign-off depending only on the $SUPERPLANE_TASK_DIR/bin/git wrapper. It's prepended to PATH (agent_task.go:106) and injects -c core.hooksPath=<task dir>/git-hooks for commit/merge (factory_commit_identity.go:478-482, 538-560).
  • If the agent's git call doesn't resolve to the wrapper (PATH rebuilt by a login shell or a tool, or an absolute /usr/bin/git), no hook runs. The commit still gets the agent identity from GIT_AUTHOR_*/GIT_COMMITTER_* env vars (factory_commit_identity.go:43-46), but it isn't signed off.
  • The Commit and Push fallback (pr_feedback_template.go:671-680) uses git commit -s only for leftover staged changes and doesn't fix commits the agent already made.
  • The Codex runner (pkg/components/runner/codex/run.js) commits through Codex's own shell, which may make bypassing the wrapper more likely. Unverified.
  • Separately, .semaphore/fix-formatting.yaml:22 runs git commit without -s.

Proposed fix

  1. Have PR-feedback and fix-checks checkouts set the hook at the repo level (git config core.hooksPath "$SUPERPLANE_TASK_DIR/git-hooks" in the checkout, or run FactoryRepoCommitSetup), so it doesn't depend on PATH.
  2. Before pushing, make sure every new commit on the branch is signed (for example git rebase --signoff over the PR's own commits), or fail the step when one is missing the trailer.
  3. Optionally keep the task PATH (non-login shell) when the Codex runner launches commands.
  4. Add -s to .semaphore/fix-formatting.yaml:22.
  5. Add a test: in the PR-feedback path, a commit made with a git binary that bypasses the wrapper still ends up signed off.

Related: #3520, #8495.

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions