diff --git a/docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md b/docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md index 8c120ef99..464fa49dd 100644 --- a/docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md +++ b/docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md @@ -46,7 +46,8 @@ Accepted > artifact — only the poller could produce them. With API-triggered pipelines, > any user with pipeline-create access on the protected branch can POST > arbitrary variables (STAGE, EVENT_TYPE, EVENT_PAYLOAD_B64, RESOURCE_KEY, -> IS_FORK, MR_AUTHOR_ID, ACTOR_ID, STATUS_IID, FULLSEND_POLL_JOB_URL). The +> IS_FORK, MR_AUTHOR_ID, ACTOR_ID, STATUS_IID, FULLSEND_POLL_JOB_URL, +> ORIGINATING_URL, REPO_FULL_NAME). The > in-job authorization gate and fork > protection read these attacker-supplied variables. Mitigation #1 is > implemented: the agent job uses the Pipelines API diff --git a/internal/poll/dispatch.go b/internal/poll/dispatch.go index d4dff4bf2..c29966741 100644 --- a/internal/poll/dispatch.go +++ b/internal/poll/dispatch.go @@ -61,6 +61,8 @@ func (p *Poller) dispatch(ctx context.Context, owner, repo, stage string, event "EVENT_PAYLOAD_B64": encoded, "RESOURCE_KEY": rk, "IS_FORK": strconv.FormatBool(isFork), + "ORIGINATING_URL": entityURL(p.gitlabURL, p.projectPath, event.Type, event.IID), + "REPO_FULL_NAME": p.projectPath, } if event.MRAuthorID != 0 { variables["MR_AUTHOR_ID"] = strconv.Itoa(event.MRAuthorID) @@ -116,6 +118,8 @@ var signedDispatchKeys = []string{ "FULLSEND_POLL_JOB_URL", "IS_FORK", "MR_AUTHOR_ID", + "ORIGINATING_URL", + "REPO_FULL_NAME", "RESOURCE_KEY", "STAGE", "STATUS_IID", diff --git a/internal/poll/dispatch_test.go b/internal/poll/dispatch_test.go index 684c4fdd8..d3077d1df 100644 --- a/internal/poll/dispatch_test.go +++ b/internal/poll/dispatch_test.go @@ -821,6 +821,8 @@ func TestComputeDispatchHMAC_MatchesPython3(t *testing.T) { "FULLSEND_POLL_JOB_URL": "https://gitlab.example.com/-/jobs/12345", "IS_FORK": "false", "MR_AUTHOR_ID": "42", + "ORIGINATING_URL": "https://gitlab.example.com/testgroup/testrepo/-/merge_requests/10", + "REPO_FULL_NAME": "testgroup/testrepo", "RESOURCE_KEY": "mr-10", "STAGE": "review", "STATUS_IID": "10", @@ -852,6 +854,94 @@ func TestComputeDispatchHMAC_MatchesPython3(t *testing.T) { } } +func TestDispatch_IncludesOriginatingURLForIssueEvent(t *testing.T) { + mc := newMockClient() + p := newTestPoller(mc, Options{}) + + event := RoutableEvent{ + Type: "issue_note", + IID: 42, + UpdatedAt: time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC), + NoteBody: "/fs-retro", + NoteID: 100, + NoteAuthorID: 88, + } + + err := p.dispatch(context.Background(), "owner", "repo", "retro", event) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + vars := mc.pipelineCalls[0].Variables + wantURL := "https://gitlab.example.com/testgroup/testrepo/-/issues/42" + if vars["ORIGINATING_URL"] != wantURL { + t.Errorf("ORIGINATING_URL: got %q, want %q", vars["ORIGINATING_URL"], wantURL) + } + if vars["REPO_FULL_NAME"] != "testgroup/testrepo" { + t.Errorf("REPO_FULL_NAME: got %q, want %q", vars["REPO_FULL_NAME"], "testgroup/testrepo") + } +} + +func TestDispatch_IncludesOriginatingURLForMREvent(t *testing.T) { + mc := newMockClient() + p := newTestPoller(mc, Options{}) + + event := RoutableEvent{ + Type: "mr_event", + IID: 10, + UpdatedAt: time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC), + MRSource: 100, + MRTarget: 100, + } + + err := p.dispatch(context.Background(), "owner", "repo", "retro", event) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + vars := mc.pipelineCalls[0].Variables + wantURL := "https://gitlab.example.com/testgroup/testrepo/-/merge_requests/10" + if vars["ORIGINATING_URL"] != wantURL { + t.Errorf("ORIGINATING_URL: got %q, want %q", vars["ORIGINATING_URL"], wantURL) + } + if vars["REPO_FULL_NAME"] != "testgroup/testrepo" { + t.Errorf("REPO_FULL_NAME: got %q, want %q", vars["REPO_FULL_NAME"], "testgroup/testrepo") + } +} + +func TestDispatch_OriginatingURLWithSubgroup(t *testing.T) { + mc := newMockClient() + p := newTestPoller(mc, Options{}) + p.projectPath = "group/sub/project" + p.gitlabURL = "https://gitlab.cee.redhat.com" + + event := RoutableEvent{ + Type: "mr_note", + IID: 7, + UpdatedAt: time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC), + NoteBody: "/fs-retro", + NoteID: 200, + NoteAuthorID: 55, + MRAuthorID: 42, + MRSource: 100, + MRTarget: 100, + } + + err := p.dispatch(context.Background(), "owner", "repo", "retro", event) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + vars := mc.pipelineCalls[0].Variables + wantURL := "https://gitlab.cee.redhat.com/group/sub/project/-/merge_requests/7" + if vars["ORIGINATING_URL"] != wantURL { + t.Errorf("ORIGINATING_URL: got %q, want %q", vars["ORIGINATING_URL"], wantURL) + } + if vars["REPO_FULL_NAME"] != "group/sub/project" { + t.Errorf("REPO_FULL_NAME: got %q, want %q", vars["REPO_FULL_NAME"], "group/sub/project") + } +} + func TestResourceKey_EntityBased(t *testing.T) { tests := []struct { event RoutableEvent diff --git a/internal/poll/state_test.go b/internal/poll/state_test.go index 8b14fbf2b..c20e46c05 100644 --- a/internal/poll/state_test.go +++ b/internal/poll/state_test.go @@ -13,10 +13,12 @@ func newTestPoller(client GitLabClient, opts Options) *Poller { opts.PipelineRef = "main" } return &Poller{ - client: client, - owner: "testgroup", - repo: "testrepo", - opts: opts, + client: client, + projectPath: "testgroup/testrepo", + owner: "testgroup", + repo: "testrepo", + gitlabURL: "https://gitlab.example.com", + opts: opts, } } diff --git a/internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml b/internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml index 318ca85de..2a45d7bbb 100644 --- a/internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml +++ b/internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml @@ -164,7 +164,7 @@ stages: echo "ERROR: FULLSEND_DISPATCH_HMAC missing — dispatch variables not signed (fail-closed)" exit 1 fi - HMAC_MESSAGE=$(printf 'ACTOR_ID=%s\nEVENT_PAYLOAD_B64=%s\nEVENT_TYPE=%s\nFULLSEND_POLL_JOB_URL=%s\nIS_FORK=%s\nMR_AUTHOR_ID=%s\nRESOURCE_KEY=%s\nSTAGE=%s\nSTATUS_IID=%s' "${ACTOR_ID:-}" "${EVENT_PAYLOAD_B64:-}" "${EVENT_TYPE:-}" "${FULLSEND_POLL_JOB_URL:-}" "${IS_FORK:-}" "${MR_AUTHOR_ID:-}" "${RESOURCE_KEY:-}" "${STAGE:-}" "${STATUS_IID:-}") + HMAC_MESSAGE=$(printf 'ACTOR_ID=%s\nEVENT_PAYLOAD_B64=%s\nEVENT_TYPE=%s\nFULLSEND_POLL_JOB_URL=%s\nIS_FORK=%s\nMR_AUTHOR_ID=%s\nORIGINATING_URL=%s\nREPO_FULL_NAME=%s\nRESOURCE_KEY=%s\nSTAGE=%s\nSTATUS_IID=%s' "${ACTOR_ID:-}" "${EVENT_PAYLOAD_B64:-}" "${EVENT_TYPE:-}" "${FULLSEND_POLL_JOB_URL:-}" "${IS_FORK:-}" "${MR_AUTHOR_ID:-}" "${ORIGINATING_URL:-}" "${REPO_FULL_NAME:-}" "${RESOURCE_KEY:-}" "${STAGE:-}" "${STATUS_IID:-}") if ! printf '%s' "${HMAC_MESSAGE}" | HMAC_SECRET="${FULLSEND_DISPATCH_SECRET}" python3 -c 'import hmac,hashlib,os,sys; expected=hmac.new(os.environ["HMAC_SECRET"].encode(),sys.stdin.read().encode(),hashlib.sha256).hexdigest(); sys.exit(0 if hmac.compare_digest(sys.argv[1],expected) else 1)' "${FULLSEND_DISPATCH_HMAC}"; then echo "ERROR: HMAC verification failed — dispatch variables may be forged (fail-closed)" exit 1 @@ -290,6 +290,14 @@ stages: esac export GITLAB_ISSUE_URL + # Extract the comment body for the retro agent. On GitHub, + # RETRO_COMMENT is set from the event payload's comment.body field; + # on GitLab the equivalent data is note_body inside EVENT_PAYLOAD_B64. + if [ "${STAGE}" = "retro" ] && [ -n "${EVENT_PAYLOAD_B64:-}" ]; then + RETRO_COMMENT=$(printf '%s' "${EVENT_PAYLOAD_B64}" | base64 -d | jq -r '.note_body // ""') + export RETRO_COMMENT + fi + # Pre-fetch prior review for the review agent — equivalent to # pre-fetch-prior-review.sh in the GitHub scaffold. Queries the # GitLab Notes API for the last bot review comment, validates