fix(#6430): pass RETRO_COMMENT in GitLab dispatch variables - #6431
fix(#6430): pass RETRO_COMMENT in GitLab dispatch variables#6431fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
The GitLab dispatch pipeline did not pass RETRO_COMMENT as a pipeline
variable. The retro harness references it via "${RETRO_COMMENT}" in
env.sandbox, and the fullsend CLI's env validation treats any ${VAR}
reference as required — causing programmatic retro runs to fail
immediately on repos that haven't re-enrolled to pick up the scaffold
extraction added in #6419.
Pass RETRO_COMMENT unconditionally in the dispatch variables map,
sourced from event.NoteBody. For note-triggered retro runs this
provides the comment text directly; for programmatic dispatches and
non-note events it is empty. Add RETRO_COMMENT to signedDispatchKeys
and the shell HMAC verifier so dispatch integrity is maintained.
Note: pre-commit could not run (sandbox network policy blocks git
fetch for hook environment setup). The post-script runs an
authoritative pre-commit check on the CI runner.
Closes #6430
|
🤖 Finished Review · ✅ Success · Started 6:39 PM UTC · Completed 6:54 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| "ORIGINATING_URL", | ||
| "REPO_FULL_NAME", | ||
| "RESOURCE_KEY", | ||
| "RETRO_COMMENT", |
There was a problem hiding this comment.
[medium] backward-compatibility
Adding RETRO_COMMENT to signedDispatchKeys changes the HMAC signature produced by the Go poller. During a rolling update where the poller is deployed before customer repos re-sync their scaffold template, the 12-key HMAC will not match the old 11-key shell verifier, causing fail-closed dispatch rejection. This follows the same pattern as the ORIGINATING_URL addition (#6418/#6419).
Suggested fix: If the transition breakage is unacceptable, consider a two-phase rollout: deploy the scaffold template change first so repos can pick it up, then add the key to signedDispatchKeys. If the team has accepted fail-closed transition breakage as policy (as with ORIGINATING_URL), no code change is needed — ensure scaffold re-sync is communicated.
| "IS_FORK": strconv.FormatBool(isFork), | ||
| "ORIGINATING_URL": entityURL(p.gitlabURL, p.projectPath, event.Type, event.IID), | ||
| "REPO_FULL_NAME": p.projectPath, | ||
| "RETRO_COMMENT": event.NoteBody, |
There was a problem hiding this comment.
[low] logic-error
RETRO_COMMENT is set unconditionally to event.NoteBody for all dispatch types, including non-retro events. No functional bug since consumers only read RETRO_COMMENT in the retro stage, but semantically misleading for non-retro pipelines.
Suggested fix: Consider gating the assignment: only set RETRO_COMMENT when stage == "retro". The HMAC computation handles missing keys by using the zero string value.
| "IS_FORK": strconv.FormatBool(isFork), | ||
| "ORIGINATING_URL": entityURL(p.gitlabURL, p.projectPath, event.Type, event.IID), | ||
| "REPO_FULL_NAME": p.projectPath, | ||
| "RETRO_COMMENT": event.NoteBody, |
There was a problem hiding this comment.
[low] injection
RETRO_COMMENT is populated from user-controlled NoteBody without sanitizing newlines. Since the HMAC message uses newline as field delimiters, embedded newlines create structural ambiguity. Not exploitable because both signer and verifier construct the same message, but weakens the integrity guarantee.
Suggested fix: Consider sanitizing newlines in NoteBody before placing it in the variables map, or base64-encode the value in the HMAC message while keeping the raw value in the pipeline variable.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
Closing — the scaffold's |
|
🤖 Finished Retro · ✅ Success · Started 7:19 PM UTC · Completed 7:32 PM UTC Commit: |
Retro: PR #6431 — unnecessary code change closed without mergePR #6431 was a code agent PR to fix #6430 (GitLab dispatch missing Timeline
Review qualityThe review agent's findings were high quality and relevant to the proposed changes. The HMAC rolling-update backward-compatibility finding (medium) was a genuine concern for the approach taken. The review correctly identified that adding a 12th key to Evidence for existing issues
Proposals filed
|
1 participant
Summary
Pass
RETRO_COMMENTas a GitLab dispatch pipeline variable so the retro harness env validation succeeds without requiring scaffold re-enrollment.The retro harness references
RETRO_COMMENTvia"${RETRO_COMMENT}"inenv.sandbox, but the GitLab dispatch did not include it as a pipeline variable. The fullsend CLI's env validation treats any${VAR}reference as required, causing programmatic retro runs to fail immediately on repos that haven't re-enrolled to pick up the scaffold extraction from #6419.Changes
RETRO_COMMENT(sourced fromevent.NoteBody) to the dispatch variables map indispatch.goRETRO_COMMENTtosignedDispatchKeysfor HMAC integrityfullsend-agent.ymlto includeRETRO_COMMENTRETRO_COMMENTis set for note events and empty for non-note eventsTesting
TestDispatch_IncludesRetroComment— verifies note body is passed asRETRO_COMMENTTestDispatch_RetroCommentEmptyForNonNoteEvent— verifies empty for non-note eventsTestSignedDispatchKeys_MatchShellTemplate— verifies Go and shell HMAC key lists matchTestComputeDispatchHMAC_MatchesPython3— verifies cross-language HMAC consistency withRETRO_COMMENTdispatch.gofunctions 86-100%, package overall 89.8%Closes #6430
Post-script verification
agent/6430-gitlab-retro-comment-env)ff93b07b4c2c41df143a95c3ca61088a210738fd..HEAD)