Skip to content

docs: add ADR 0043 for GitLab support via webhook bridge - #1816

Closed
ggallen wants to merge 1 commit into
fullsend-ai:mainfrom
ggallen:docs/adr-0043-gitlab-support
Closed

docs: add ADR 0043 for GitLab support via webhook bridge#1816
ggallen wants to merge 1 commit into
fullsend-ai:mainfrom
ggallen:docs/adr-0043-gitlab-support

Conversation

@ggallen

@ggallen ggallen commented Jun 2, 2026

Copy link
Copy Markdown
Member

Add ADR 0043 documenting the webhook bridge architecture for GitLab support, replacing the broader ADR 0028 approach with a focused, incrementally deployable design.

Key decisions:

  • Webhook bridge Cloud Function translates GitLab webhooks to pipeline triggers with hardcoded ref=main for security
  • OIDC via Workload Identity Federation for bridge-to-mint auth
  • Project Access Tokens for per-role GitLab credentials
  • Defense-in-depth: protected CI/CD variables, per-project webhook secrets, payload validation

Also adds companion implementation document (docs/problems/gitlab-support.md) with phased rollout plan and updates ADR 0028 status to reference its successor.

Closes #1964

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

Site preview

Preview: https://8becfb24-site.fullsend-ai.workers.dev

Commit: 058e034b1e5e3e71a9fa0ec2bd0400c34faedd6b

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [auth-bypass] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:245 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses FULLSEND_DISPATCH_TOKEN can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing per-project webhook secret validation. The document thoroughly acknowledges this as an accepted risk (ADR 0043 Risks section, implementation plan trust boundary comments). Anchored from prior review.
    Remediation: Consider implementing the HMAC-based payload signing mentioned in the threat model note before production deployment.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:272 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days with the rotation cadence). This is an explicitly acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). Improved mitigations in this revision: 120-day creation expiry with 90-day rotation cadence (30-day buffer), CI_DEBUG_TRACE guards in both dispatch and child pipelines, PAT usage audit alerting elevated to required, and mandatory pre-production prototype gate for the group-level bot account approach (Phase 6). Anchored from prior review.
    Remediation: Complete the group-level bot account prototype before finalizing the GitLab implementation.

Low

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard now appears in both the dispatch pipeline's validate-webhook job and the stage pipeline template (triage.yml). The residual risk — GitLab prints variable values before user scripts execute — is an inherent platform limitation that the document correctly identifies. Defense-in-depth layers (install-time hard error, analyze-time hard error, script-level guard, project-level restriction recommendation) are appropriate. Anchored from prior review; downgraded from medium.

  • [privilege-escalation] docs/plans/gitlab-support.md:1460 — The fullsend orchestrator role is assigned Maintainer-level PAT access. A compromised orchestrator PAT could modify protected branch settings, CI/CD variables, and project settings — undermining the ref=main security invariant. The current revision includes a security note documenting this as the highest-sensitivity credential. Anchored from prior review.
    Remediation: Evaluate whether Developer access with specific API permissions would suffice for orchestrator operations.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section is normalized to space format (ADR 0005, ADR 0017) but body text retains hyphenated format in sections not touched by this PR, creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — The References section is normalized to space format but body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030), creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

Info

  • [scope-alignment] PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update to Superseded, and cross-reference updates across the documentation. No scope creep detected.

  • [design-observation] All prior correctness findings have been resolved: stage-to-role mapping now uses canonical coder name (code|fix → coder in both ADR 0043 and dispatch template), retro/prioritize correctly share the fullsend Maintainer role matching GitHub, config.yaml lookup uses yq bracket notation eliminating dotpath collision, max_personal_access_token_lifetime validation added to install flow, diagnostic logging added for FULLSEND_REVIEW_BOT_USERNAME, trailing slash handling present, escaping scheme differences documented.

Previous run

Review

Findings

High

  • [internal-consistency] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The implementation plan and ADR use "code" as a role name in the credential model (e.g., "Role mapping: triage → Reporter, code → Developer"). However, the codebase's canonical role name is "coder", not "code" — see internal/config/config.go:58 (ValidRoles() returns "coder"), internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml:276 (code|fix) STAGE_ROLE="coder"), and internal/scaffold/fullsend-repo/.github/actions/mint-token/action.yml. The GitLab CI/CD template example (triage.yml) mints with "role":"triage" (stage name = role name), but no stage-to-role mapping equivalent to GitHub's code|fix → coder exists in the GitLab templates. When the code stage runs and requests role: code from the mint, ValidRoles() will reject it as unrecognized, and the Secret Manager key lookup will use the wrong name.
    Remediation: Either (a) add the stage-to-role mapping (code|fix → coder, retro|prioritize → fullsend) to the GitLab dispatch or stage pipeline templates, consistent with GitHub's dispatch.yml:274-278; or (b) update the role names in the ADR and plan to use the codebase's canonical names. Option (a) is strongly preferred since it preserves consistency with the existing credential model.

Medium

  • [internal-consistency] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The ADR's credential model assigns separate PATs for retro → Reporter and prioritize → Developer. However, on GitHub both stages map to the fullsend role (Maintainer credentials) — see dispatch.yml:277 (retro|prioritize) STAGE_ROLE="fullsend"). If these agents require Maintainer-level permissions (e.g., modifying project settings, managing labels, closing issues in bulk), the downgraded Reporter/Developer PATs will cause runtime failures. The plan does not document why the GitLab permission mapping deliberately diverges from GitHub's credential model.
    Remediation: Document whether retro and prioritize agents actually need Maintainer-level access. If they share the fullsend (Maintainer) role on GitHub because of permission requirements, the GitLab mapping should match. If the GitHub mapping is overly permissive and Reporter/Developer suffices, document this as an intentional improvement.

  • [edge-case] docs/plans/gitlab-support.md:1153 — The validate-webhook script converts SOURCE_PROJECT to a config.yaml lookup key via tr '/' '.', then queries yq ".repos.\"${CONFIG_KEY}\".enabled". The existing OrgConfig.Repos map (internal/config/config.go:82) uses repo names as keys (e.g., my-repo). For GitLab subgroups, if the config stores just the project name — matching the current GitHub convention — the tr-transformed full project path (e.g., myorg.subgroup.project) will never match a bare project-name key. The plan needs to specify the config.yaml key format for GitLab repos.
    Remediation: Specify whether GitLab repos are keyed by full project path (myorg/subgroup/project) or by project name (project) in config.yaml, and ensure the validate-webhook script's lookup is consistent.

  • [auth-bypass] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:245 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses FULLSEND_DISPATCH_TOKEN can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing per-project webhook secret validation. The document thoroughly acknowledges this as an accepted risk (ADR 0043 Risks section, implementation plan trust boundary comments). Anchored from prior review.
    Remediation: Consider implementing the HMAC-based payload signing mentioned in the threat model note before production deployment.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:272 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days with the rotation cadence). This is an explicitly acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). Improved mitigations in this revision: 120-day creation expiry with 90-day rotation cadence (30-day buffer), CI_DEBUG_TRACE guards in both dispatch and child pipelines, PAT usage audit alerting elevated to required, and mandatory pre-production prototype gate for the group-level bot account approach (Phase 6). Anchored from prior review.
    Remediation: Complete the group-level bot account prototype before finalizing the GitLab implementation.

Low

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard now appears in both the dispatch pipeline's validate-webhook job and the stage pipeline template (triage.yml). The residual risk — GitLab prints variable values before user scripts execute — is an inherent platform limitation that the document correctly identifies. Defense-in-depth layers (install-time hard error, analyze-time hard error, script-level guard, project-level restriction recommendation) are appropriate. Anchored from prior review; downgraded from medium.

  • [privilege-escalation] docs/plans/gitlab-support.md:1460 — The fullsend orchestrator role is assigned Maintainer-level PAT access. A compromised orchestrator PAT could modify protected branch settings, CI/CD variables, and project settings — undermining the ref=main security invariant. The current revision includes a security note documenting this as the highest-sensitivity credential. Anchored from prior review.
    Remediation: Evaluate whether Developer access with specific API permissions would suffice for orchestrator operations.

  • [edge-case] docs/plans/gitlab-support.md:1244 — The <!-- fullsend:changes-requested --> auto-fix trigger requires FULLSEND_REVIEW_BOT_USERNAME to be non-empty. The current revision includes a diagnostic log message when unset, resolving the prior finding about silent failure. However, the install flow (Phase 5, step 11a) warns but does not error when not provided. Anchored from prior review.
    Remediation: Consider making FULLSEND_REVIEW_BOT_USERNAME a required parameter during fullsend admin install for GitLab.

  • [edge-case] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The credential model does not mention validating against the self-hosted GitLab instance's max_personal_access_token_lifetime setting. The implementation plan (line 269) now addresses this with a validation step during install. Anchored from prior review.

  • [edge-case] docs/plans/gitlab-support.md:579 — The GitLab forge client constructor appends /api/v4 to the baseURL. The implementation plan now includes strings.TrimRight(baseURL, "/") to handle trailing slashes. Anchored from prior review — resolved.

  • [internal-consistency] docs/plans/gitlab-support.md:1147 — The validate-webhook script uses tr '/' '.' for config.yaml lookup while Secret Manager uses _ as the path separator. The document now includes a clarifying note about the two distinct escaping schemes. Anchored from prior review — resolved.

  • [injection] docs/plans/gitlab-support.md:1152 — The config.yaml lookup uses tr '/' '.' to convert project paths to yq dotpath notation, creating a collision risk for paths like group.name/project and group/name.project (both map to group.name.project). A malicious actor controlling project naming could exploit this to have webhook events treated as originating from a different enrolled project.
    Remediation: Use yq bracket notation (e.g., .repos["myorg/project"].enabled) instead of dotpath notation to avoid the escaping ambiguity.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section is normalized to space format (ADR 0005, ADR 0017) but body text retains hyphenated format in sections not touched by this PR, creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — The References section is normalized to space format but body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030), creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

Info

  • [scope-alignment] PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update to Superseded, and cross-reference updates across the documentation. No scope creep detected.

  • [design-observation] All prior correctness findings have been resolved: diagnostic logging added for FULLSEND_REVIEW_BOT_USERNAME, MinimalChanges includes Title/Description fields, RESOURCE_KEY forwarded in trigger-stage variables, normalizeClaims uses string as primary case for ref_protected, generate-child-config includes $STAGE guard, credential rotation uses 120/90-day intervals with 30-day buffer, CI_DEBUG_TRACE guard added to stage pipeline templates, PAT usage audit alerting elevated to required.

Previous run

Review

Findings

High

  • [internal-consistency] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The implementation plan and ADR use "code" as a role name in the credential model (e.g., "Role mapping: triage → Reporter, code → Developer"). However, the codebase's canonical role name is "coder", not "code" — see internal/config/config.go:58 (ValidRoles() returns "coder"), internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml:276 (code|fix) STAGE_ROLE="coder"), and internal/scaffold/fullsend-repo/.github/actions/mint-token/action.yml. The GitLab CI/CD template example (triage.yml) mints with "role":"triage" (stage name = role name), but no stage-to-role mapping equivalent to GitHub's code|fix → coder exists in the GitLab templates. When the code stage runs and requests role: code from the mint, ValidRoles() will reject it as unrecognized, and the Secret Manager key lookup will use the wrong name.
    Remediation: Either (a) add the stage-to-role mapping (code|fix → coder, retro|prioritize → fullsend) to the GitLab dispatch or stage pipeline templates, consistent with GitHub's dispatch.yml:274-278; or (b) update the role names in the ADR and plan to use the codebase's canonical names. Option (a) is strongly preferred since it preserves consistency with the existing credential model.

Medium

  • [internal-consistency] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The ADR's credential model assigns separate PATs for retro → Reporter and prioritize → Developer. However, on GitHub both stages map to the fullsend role (Maintainer credentials) — see dispatch.yml:277 (retro|prioritize) STAGE_ROLE="fullsend"). If these agents require Maintainer-level permissions (e.g., modifying project settings, managing labels, closing issues in bulk), the downgraded Reporter/Developer PATs will cause runtime failures. The plan does not document why the GitLab permission mapping deliberately diverges from GitHub's credential model.
    Remediation: Document whether retro and prioritize agents actually need Maintainer-level access. If they share the fullsend (Maintainer) role on GitHub because of permission requirements, the GitLab mapping should match. If the GitHub mapping is overly permissive and Reporter/Developer suffices, document this as an intentional improvement.

  • [edge-case] docs/plans/gitlab-support.md:1153 — The validate-webhook script converts SOURCE_PROJECT to a config.yaml lookup key via tr '/' '.', then queries yq ".repos.\"${CONFIG_KEY}\".enabled". The existing OrgConfig.Repos map (internal/config/config.go:82) uses repo names as keys (e.g., my-repo). For GitLab subgroups, if the config stores just the project name — matching the current GitHub convention — the tr-transformed full project path (e.g., myorg.subgroup.project) will never match a bare project-name key. The plan needs to specify the config.yaml key format for GitLab repos.
    Remediation: Specify whether GitLab repos are keyed by full project path (myorg/subgroup/project) or by project name (project) in config.yaml, and ensure the validate-webhook script's lookup is consistent.

  • [auth-bypass] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:245 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses FULLSEND_DISPATCH_TOKEN can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing per-project webhook secret validation. The document thoroughly acknowledges this as an accepted risk (ADR 0043 Risks section, implementation plan trust boundary comments). Anchored from prior review.
    Remediation: Consider implementing the HMAC-based payload signing mentioned in the threat model note before production deployment.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:272 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days with the rotation cadence). This is an explicitly acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). Improved mitigations in this revision: 120-day creation expiry with 90-day rotation cadence (30-day buffer), CI_DEBUG_TRACE guards in both dispatch and child pipelines, PAT usage audit alerting elevated to required, and mandatory pre-production prototype gate for the group-level bot account approach (Phase 6). Anchored from prior review.
    Remediation: Complete the group-level bot account prototype before finalizing the GitLab implementation.

Low

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard now appears in both the dispatch pipeline's validate-webhook job and the stage pipeline template (triage.yml). The residual risk — GitLab prints variable values before user scripts execute — is an inherent platform limitation that the document correctly identifies. Defense-in-depth layers (install-time hard error, analyze-time hard error, script-level guard, project-level restriction recommendation) are appropriate. Anchored from prior review; downgraded from medium.

  • [privilege-escalation] docs/plans/gitlab-support.md:1460 — The fullsend orchestrator role is assigned Maintainer-level PAT access. A compromised orchestrator PAT could modify protected branch settings, CI/CD variables, and project settings — undermining the ref=main security invariant. The current revision includes a security note documenting this as the highest-sensitivity credential. Anchored from prior review.
    Remediation: Evaluate whether Developer access with specific API permissions would suffice for orchestrator operations.

  • [injection] docs/plans/gitlab-support.md:1152 — The config.yaml lookup uses tr '/' '.' to convert project paths to yq dotpath notation, creating a collision risk for paths like group.name/project and group/name.project (both map to group.name.project). A malicious actor controlling project naming could exploit this to have webhook events treated as originating from a different enrolled project.
    Remediation: Use yq bracket notation (e.g., .repos["myorg/project"].enabled) instead of dotpath notation to avoid the escaping ambiguity.

  • [edge-case] docs/plans/gitlab-support.md:1244 — The <!-- fullsend:changes-requested --> auto-fix trigger requires FULLSEND_REVIEW_BOT_USERNAME to be non-empty. The current revision includes a diagnostic log message when unset, resolving the prior finding about silent failure. However, the install flow (Phase 5, step 11a) warns but does not error when not provided. Anchored from prior review.
    Remediation: Consider making FULLSEND_REVIEW_BOT_USERNAME a required parameter during fullsend admin install for GitLab.

  • [edge-case] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The credential model does not mention validating against the self-hosted GitLab instance's max_personal_access_token_lifetime setting. The implementation plan (line 269) now addresses this with a validation step during install. Anchored from prior review.

  • [edge-case] docs/plans/gitlab-support.md:579 — The GitLab forge client constructor appends /api/v4 to the baseURL. The implementation plan now includes strings.TrimRight(baseURL, "/") to handle trailing slashes. Anchored from prior review — resolved.

  • [internal-consistency] docs/plans/gitlab-support.md:1147 — The validate-webhook script uses tr '/' '.' for config.yaml lookup while Secret Manager uses _ as the path separator. The document now includes a clarifying note about the two distinct escaping schemes. Anchored from prior review — resolved.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section is normalized to space format (ADR 0005, ADR 0017) but body text retains hyphenated format in sections not touched by this PR, creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — The References section is normalized to space format but body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030), creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

Info

  • [scope-alignment] PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update to Superseded, and cross-reference updates across the documentation. No scope creep detected.

  • [design-observation] All prior correctness findings have been resolved: diagnostic logging added for FULLSEND_REVIEW_BOT_USERNAME, MinimalChanges includes Title/Description fields, RESOURCE_KEY forwarded in trigger-stage variables, normalizeClaims uses string as primary case for ref_protected, generate-child-config includes $STAGE guard, credential rotation uses 120/90-day intervals with 30-day buffer, CI_DEBUG_TRACE guard added to stage pipeline templates, PAT usage audit alerting elevated to required.

Previous run (2)

Review

Findings

Medium

  • [auth-bypass] docs/plans/gitlab-support.md:1123 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses FULLSEND_DISPATCH_TOKEN can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing per-project webhook secret validation. The document now thoroughly acknowledges this as an accepted risk (lines 1126–1134): trigger token possession equates to Maintainer trust, and Maintainers could also modify pipeline code directly. The HMAC-based alternative is mentioned but deferred. The trust boundary documentation is comprehensive; this finding is retained to ensure human reviewers are aware of the design trade-off. Anchored from prior review.
    Remediation: Consider implementing the HMAC-based payload signing mentioned in the threat model note before production deployment, or explicitly add this bypass scenario to the threat model in ADR 0043's Risks section.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:273 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days with the rotation cadence). This is an explicitly acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). The current revision has improved mitigations: 120-day creation expiry with 90-day rotation cadence (30-day buffer), CI_DEBUG_TRACE guards in both dispatch and child pipelines, and PAT usage audit alerting elevated from "recommended" to required. The fundamental exposure window remains larger than GitHub's model. Anchored from prior review.
    Remediation: Prototype the group-level bot account approach (ADR 0043 Open Questions) before finalizing the GitLab implementation. This could restore on-demand, short-lived token generation analogous to the GitHub model.

Low

  • [edge-case] docs/plans/gitlab-support.md:815 — The <!-- fullsend:changes-requested --> auto-fix trigger requires FULLSEND_REVIEW_BOT_USERNAME to be non-empty. The current revision now includes a diagnostic log message when the variable is unset, resolving the prior finding about silent failure. However, the install flow (Phase 5, step 11a) warns but does not error when the username is not provided, so operators may still miss the configuration during initial setup. Anchored from prior review.
    Remediation: Consider making FULLSEND_REVIEW_BOT_USERNAME a required parameter during fullsend admin install for GitLab (with a skip flag for orgs that don't use the review bot), rather than a warn-only prompt.

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard now appears in both the dispatch pipeline's validate-webhook job and the stage pipeline template (triage.yml). This resolves the prior finding about the guard being absent from child pipelines. The residual risk — GitLab prints variable values before user scripts execute — is an inherent platform limitation that the document correctly identifies. The defense-in-depth layers (install-time hard error, analyze-time hard error, script-level guard, project-level restriction recommendation) are appropriate. Anchored from prior review; downgraded from medium — the actionable gap is resolved.

  • [privilege-escalation] docs/plans/gitlab-support.md:1446 — The fullsend orchestrator role is assigned Maintainer-level PAT access. A compromised orchestrator PAT could modify protected branch settings, CI/CD variables, and project settings — undermining the ref=main security invariant. The current revision includes a security note documenting this as the highest-sensitivity credential. Anchored from prior review.
    Remediation: Evaluate whether Developer access with specific API permissions would suffice for orchestrator operations.

  • [internal-consistency] docs/plans/gitlab-support.md:1144 — The validate-webhook script uses tr '/' '.' to convert the SOURCE_PROJECT path into a yq config key, but the Secret Manager naming convention uses _ as the path separator replacement (e.g., fullsend-myorg_subgroup--my-project--triage-pat). These are two different escaping schemes for the same project path component: /. for config.yaml keys, /_ for Secret Manager keys. While they serve different purposes, the document doesn't clarify this distinction, which could confuse implementers.
    Remediation: Add a brief note in the validate-webhook script comment explaining that the . escaping is specific to yq's dotpath notation for config.yaml lookup, distinct from the _ escaping used for Secret Manager secret IDs.

  • [edge-case] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:268 — The credential model specifies 120-day creation expiry but does not mention validating against the self-hosted GitLab instance's max_personal_access_token_lifetime setting, which administrators can configure to shorter values. If the instance's maximum is less than 120 days, PAT creation would fail during install.
    Remediation: Add a validation step during fullsend admin install that checks the instance's max_personal_access_token_lifetime and adjusts the creation expiry accordingly (or errors with a message to adjust the instance setting).

  • [edge-case] docs/plans/gitlab-support.md:579 — The GitLab forge client constructor appends /api/v4 to the baseURL. If an operator passes a --gitlab-url value with a trailing slash (e.g., https://gitlab.example.com/), the resulting URL would have a double slash. The constructor should strip trailing slashes from baseURL before appending.
    Remediation: Add strings.TrimRight(baseURL, "/") before appending /api/v4.

  • [design-direction] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:354 — The Open Questions section documents a decision criteria for revisiting the PAT storage model (on-demand generation via group-level tokens). While the PAT storage approach is accepted as the current design, the mandatory prototype requirement before production creates implementation-phase gating that would benefit from explicit linkage to the implementation plan's phasing.
    Remediation: Add a note in the implementation plan (Phase 3 or Phase 6) explicitly referencing the Open Questions prototype requirement as a pre-production gate.

  • [stale-doc] docs/problems/platform-nativeness.md:134 — The open question "Is the forge abstraction worth the cost at this stage?" states "Fullsend's only concrete implementation is GitHub." While this remains technically true (no GitLab code exists yet), ADR 0043 and its companion implementation plan significantly shift the context — the forge abstraction's value is now validated by a complete second-forge architecture. The question's framing could be updated to reflect this.
    Remediation: Update to acknowledge that the GitLab architecture is designed (ADR 0043) even though implementation has not started, shifting the question from whether forge-neutrality is premature to whether the current abstraction layer adequately serves both forges.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section is normalized to space format (ADR 0005, ADR 0017) but body text retains hyphenated format in sections not touched by this PR, creating intra-file inconsistency. Pre-existing issue made more visible by targeted updates. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — The References section is normalized to space format but body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030), creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

Info

  • [scope-alignment] PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update to Superseded, and cross-reference updates across the documentation. No scope creep detected.

  • [design-observation] All prior correctness findings have been resolved: diagnostic logging added for FULLSEND_REVIEW_BOT_USERNAME, MinimalChanges includes Title/Description fields, RESOURCE_KEY forwarded in trigger-stage variables, normalizeClaims uses string as primary case for ref_protected, generate-child-config includes $STAGE guard, credential rotation uses 120/90-day intervals with 30-day buffer.

  • [design-observation] PAT usage audit alerting has been elevated from "recommended" to required, with fullsend admin analyze treating missing audit log streaming as a hard error. This is a meaningful security improvement.

Previous run (3)

Review

Findings

Medium

  • [auth-bypass] docs/plans/gitlab-support.md:1123 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses FULLSEND_DISPATCH_TOKEN can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing per-project webhook secret validation. The document now thoroughly acknowledges this as an accepted risk (lines 1126–1134): trigger token possession equates to Maintainer trust, and Maintainers could also modify pipeline code directly. The HMAC-based alternative is mentioned but deferred. The trust boundary documentation is comprehensive; this finding is retained to ensure human reviewers are aware of the design trade-off.
    Remediation: Consider implementing the HMAC-based payload signing mentioned in the threat model note before production deployment, or explicitly add this bypass scenario to the threat model in ADR 0043's Risks section.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:273 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days with the rotation cadence). This is an explicitly acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). The current revision has improved mitigations: 120-day creation expiry with 90-day rotation cadence (30-day buffer), CI_DEBUG_TRACE guards in both dispatch and child pipelines, and PAT usage audit alerting elevated from "recommended" to required. The fundamental exposure window remains larger than GitHub's model.
    Remediation: Prototype the group-level bot account approach (ADR 0043 Open Questions) before finalizing the GitLab implementation. This could restore on-demand, short-lived token generation analogous to the GitHub model.

Low

  • [edge-case] docs/plans/gitlab-support.md:815 — The <!-- fullsend:changes-requested --> auto-fix trigger requires FULLSEND_REVIEW_BOT_USERNAME to be non-empty. If not set during install (the install flow warns but does not error), the auto-fix trigger is silently disabled with no runtime indication in dispatch logs. Operators may not realize why auto-fix is not triggering.
    Remediation: Log a message in the determine-stage script when BOT_USERNAME is empty and the marker is present, so operators can diagnose the silent disable.

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard now appears in both the dispatch pipeline's validate-webhook job and the stage pipeline template (triage.yml). This resolves the prior finding about the guard being absent from child pipelines. The residual risk — GitLab prints variable values before user scripts execute — is an inherent platform limitation that the document correctly identifies. The defense-in-depth layers (install-time hard error, analyze-time hard error, script-level guard, project-level restriction recommendation) are appropriate. Anchored from prior review; downgraded from medium — the actionable gap is resolved.

  • [privilege-escalation] docs/plans/gitlab-support.md:1446 — The fullsend orchestrator role is assigned Maintainer-level PAT access. A compromised orchestrator PAT could modify protected branch settings, CI/CD variables, and project settings — undermining the ref=main security invariant. This is a broader blast radius than other per-role PATs (Reporter/Developer).
    Remediation: Document that the orchestrator Maintainer PAT is the highest-sensitivity credential. Evaluate whether Developer access with specific API permissions would suffice for orchestrator operations.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section is normalized to space format (ADR 0005, ADR 0017) but body text retains hyphenated format in sections not touched by this PR, creating intra-file inconsistency. Pre-existing issue made more visible by targeted updates. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — The References section is normalized to space format but body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030), creating intra-file inconsistency. Pre-existing issue. Anchored from prior review.

Info

  • [scope-alignment] PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update to Superseded, and cross-reference updates across the documentation. No scope creep detected.

  • [design-observation] All five prior correctness findings have been resolved: ref_protected type assertion ordering corrected (string as primary branch), duplicated sentence removed, org/repos[0] consistency validation added, SSH URL limitation documented, and branch name/protection validation in analyze documented.

  • [design-observation] PAT usage audit alerting has been elevated from "recommended" to required, with fullsend admin analyze treating missing audit log streaming as a hard error. This is a meaningful security improvement.

Previous run (4)

Review

Findings

Medium

  • [api-contract] docs/plans/gitlab-support.md:938 — The normalizeClaims code snippet handles ref_protected with case bool: as the primary branch and case string: as a defensive fallback that logs a warning. However, GitLab OIDC tokens serialize ref_protected as a JSON string ("true"/"false"), not a JSON boolean. Since encoding/json unmarshals into map[string]interface{}, a JSON string decodes as Go string, not bool. The case bool: branch would never execute, and every valid GitLab OIDC request would trigger the spurious WARN: ref_protected claim is a string log, potentially masking real issues in monitoring.
    Remediation: Make case string: the primary branch (no warning) and case bool: the defensive fallback. Alternatively, verify the actual serialization format from a GitLab 17.0+ instance before implementation.

  • [auth-bypass] docs/plans/gitlab-support.md:1123 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses the FULLSEND_DISPATCH_TOKEN (pipeline trigger token) can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing webhook secret validation entirely. The document acknowledges this trust boundary (trigger token is a protected CI/CD variable accessible to Maintainers), but the control is a convention, not a cryptographic guarantee.
    Remediation: Consider having the bridge sign the payload (e.g., HMAC with a shared secret in Secret Manager, not a CI/CD variable) and verifying the signature in the dispatch pipeline. If the current approach is retained, explicitly add this bypass scenario to the threat model.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:273 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days until rotation). This is an acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). A compromised agent sandbox or pipeline log leak would expose a credential valid for up to 90 days with role-level project access. The ADR documents this but the implementation plan lacks compensating controls beyond rotation.
    Remediation: Before production deployment, prioritize the open question about on-demand PAT generation via group-level tokens (ADR 0043 "Open Questions"). Add PAT usage audit alerting as a hard requirement (not just "recommended"). Add the CI_DEBUG_TRACE guard to stage pipeline templates as well (see next finding).

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard appears only in the dispatch pipeline's validate-webhook job. Stage pipeline templates (triage.yml, code.yml, etc.) do not repeat this guard. When CI_DEBUG_TRACE is enabled, GitLab prints all CI/CD variable values — including the minted PAT stored in FULLSEND_FORGE_TOKEN — to pipeline logs before user scripts execute. The dispatch guard cannot protect child pipelines that run in separate jobs.
    Remediation: Add the CI_DEBUG_TRACE guard to stage pipeline templates. Add a fullsend admin analyze periodic check that alerts if debug tracing has been re-enabled post-install.

Low

  • [internal-consistency] docs/plans/gitlab-support.md:1097 — The Security Considerations section contains a duplicated sentence: "Storing them as CI/CD variables was ADR 0028's approach, explicitly rejected by ADR 0043" appears twice in the same paragraph (once as part of a longer sentence, once standalone with "Alternative 3" added). Remove the duplicate.

  • [edge-case] docs/plans/gitlab-support.md:994GitLabCredentialBackend.MintToken accepts both an org parameter and repos[0] (full project path from which the group is extracted). If these disagree, the function uses the wrong group for the Secret Manager key. Document which is authoritative and add a consistency check.

  • [edge-case] docs/plans/gitlab-support.md:516DetectForge uses url.Parse to extract the hostname, but Git remote URLs are frequently in SSH format (git@gitlab.com:group/project.git), which is not a valid URL. Auto-detection will fail silently for SSH remotes. Document that --forge is required for SSH remotes, or add SSH URL parsing.

  • [edge-case] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:241 — The bridge hardcodes ref=main. If a Maintainer renames the default branch after installation, the security invariant could break if an unprotected main branch is re-created. Ensure fullsend admin analyze explicitly checks both that the default branch is still main and that it is still protected.

Previous run (5)

Review

Findings

Medium

  • [api-contract] docs/plans/gitlab-support.md:938 — The normalizeClaims code snippet handles ref_protected with case bool: as the primary branch and case string: as a defensive fallback that logs a warning. However, GitLab OIDC tokens serialize ref_protected as a JSON string ("true"/"false"), not a JSON boolean. Since encoding/json unmarshals into map[string]interface{}, a JSON string decodes as Go string, not bool. The case bool: branch would never execute, and every valid GitLab OIDC request would trigger the spurious WARN: ref_protected claim is a string log, potentially masking real issues in monitoring.
    Remediation: Make case string: the primary branch (no warning) and case bool: the defensive fallback. Alternatively, verify the actual serialization format from a GitLab 17.0+ instance before implementation.

  • [auth-bypass] docs/plans/gitlab-support.md:1123 — The dispatch pipeline trusts a WEBHOOK_VALIDATED=true trigger variable as proof that the bridge validated the webhook secret. Anyone who possesses the FULLSEND_DISPATCH_TOKEN (pipeline trigger token) can call the Pipeline Trigger API directly and set WEBHOOK_VALIDATED=true, bypassing webhook secret validation entirely. The document acknowledges this trust boundary (trigger token is a protected CI/CD variable accessible to Maintainers), but the control is a convention, not a cryptographic guarantee.
    Remediation: Consider having the bridge sign the payload (e.g., HMAC with a shared secret in Secret Manager, not a CI/CD variable) and verifying the signature in the dispatch pipeline. If the current approach is retained, explicitly add this bypass scenario to the threat model.

  • [data-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:273 — GitLab PATs are returned directly by the mint as long-lived credentials (up to 90 days until rotation). This is an acknowledged security regression from GitHub's model (1-hour TTL, repo-scoped installation tokens). A compromised agent sandbox or pipeline log leak would expose a credential valid for up to 90 days with role-level project access. The ADR documents this but the implementation plan lacks compensating controls beyond rotation.
    Remediation: Before production deployment, prioritize the open question about on-demand PAT generation via group-level tokens (ADR 0043 "Open Questions"). Add PAT usage audit alerting as a hard requirement (not just "recommended"). Add the CI_DEBUG_TRACE guard to stage pipeline templates as well (see next finding).

  • [data-exposure] docs/plans/gitlab-support.md:1096 — The CI_DEBUG_TRACE guard appears only in the dispatch pipeline's validate-webhook job. Stage pipeline templates (triage.yml, code.yml, etc.) do not repeat this guard. When CI_DEBUG_TRACE is enabled, GitLab prints all CI/CD variable values — including the minted PAT stored in FULLSEND_FORGE_TOKEN — to pipeline logs before user scripts execute. The dispatch guard cannot protect child pipelines that run in separate jobs.
    Remediation: Add the CI_DEBUG_TRACE guard to stage pipeline templates. Add a fullsend admin analyze periodic check that alerts if debug tracing has been re-enabled post-install.

Low

  • [internal-consistency] docs/plans/gitlab-support.md:1097 — The Security Considerations section contains a duplicated sentence: "Storing them as CI/CD variables was ADR 0028's approach, explicitly rejected by ADR 0043" appears twice in the same paragraph (once as part of a longer sentence, once standalone with "Alternative 3" added). Remove the duplicate.

  • [edge-case] docs/plans/gitlab-support.md:994GitLabCredentialBackend.MintToken accepts both an org parameter and repos[0] (full project path from which the group is extracted). If these disagree, the function uses the wrong group for the Secret Manager key. Document which is authoritative and add a consistency check.

  • [edge-case] docs/plans/gitlab-support.md:516DetectForge uses url.Parse to extract the hostname, but Git remote URLs are frequently in SSH format (git@gitlab.com:group/project.git), which is not a valid URL. Auto-detection will fail silently for SSH remotes. Document that --forge is required for SSH remotes, or add SSH URL parsing.

  • [edge-case] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:241 — The bridge hardcodes ref=main. If a Maintainer renames the default branch after installation, the security invariant could break if an unprotected main branch is re-created. Ensure fullsend admin analyze explicitly checks both that the default branch is still main and that it is still protected.

Previous run (6)

Review

Findings

Medium

  • [internal-inconsistency] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md:123 — Webhook token storage location is contradicted between ADR 0043 and its companion implementation plan. ADR 0043 Defense in depth point 3 states webhook secrets are "stored as a protected CI/CD variable in .fullsend (variable name: WEBHOOK_TOKEN_<sha256(project_path)>)." The companion implementation plan (docs/plans/gitlab-support.md) explicitly states the opposite in three places: Phase 2 deployment ("Webhook tokens loaded from Secret Manager with a TTL cache"), Phase 5 step 12 ("Generate and store webhook secret token in Secret Manager"), and Security Considerations ("webhook secrets are NOT stored as CI/CD variables in .fullsend"). The implementation plan also calls out that CI/CD variable storage was ADR 0028's approach, "explicitly rejected by ADR 0043" — yet the ADR itself still describes the CI/CD variable storage pattern. This contradiction could mislead implementers about the intended credential storage architecture.
    Remediation: Update ADR 0043 Defense in depth point 3 to state that webhook secrets are stored in Secret Manager (consistent with the implementation plan's design), not as CI/CD variables. The implementation plan's version (Secret Manager + bridge TTL cache) appears to be the intended design.

Low

  • [edge-case-correctness] docs/plans/gitlab-support.md — The normalizeClaims pseudocode extracts GitLab's ref_protected claim using a Go boolean type assertion (v.(bool)) with a silent fallback to false. If any GitLab version near the 17.0 minimum serializes this claim as a string ("true"/"false") rather than a JSON boolean, the type assertion would silently fail, causing the mint to reject all requests with a confusing "did not run on a protected ref" error. Consider adding a string-to-bool fallback with a warning log.

  • [pattern-inconsistency] README.md:45-49 — Pre-existing hyphenated ADR references (ADR-0038, ADR-0046) in unchanged lines create inconsistency with the space-separated format (ADR 0043) used in the PR's new content. Anchored from prior review.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section was normalized to space format by this PR, but body text retains hyphenated format (ADR-0017, ADR-0025, etc.), creating intra-file inconsistency. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — References section partially normalized but body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030). Anchored from prior review.

  • [pattern-inconsistency] docs/problems/gitlab-implementation.md — Retains hyphenated ADR references throughout. File is now marked superseded, so this is low priority. Anchored from prior review.

Info

  • [scope-alignment] PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update to "Superseded", and cross-reference updates across the documentation.

  • [design-observation] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md — The WEBHOOK_VALIDATED=true trust boundary, CI_DEBUG_TRACE guard limitation, and PAT scope regression are all explicitly acknowledged with thorough rationale and mitigations. The CI_DEBUG_TRACE behavior ambiguity from the prior review has been resolved — the implementation plan now explicitly states fullsend admin analyze treats it as a hard error with non-zero exit. The duplicate step numbering in GitLabCredentialBackend.MintToken from the prior review has also been fixed.

Previous run (7)

Review

Findings

Low

  • [internal-inconsistency] docs/plans/gitlab-support.md — Webhook token storage location is stated inconsistently: the bridge deployment section says tokens are "loaded from Secret Manager with a TTL cache" while the install flow (Phase 5 step 12) and Security Considerations say they are stored "as a protected CI/CD variable in .fullsend." Since the pipeline never reads the raw token (it trusts WEBHOOK_VALIDATED=true from the bridge), storing webhook tokens as CI/CD variables appears unnecessary and increases exposure surface. Clarify whether both storage locations are used, and if so, why.

  • [internal-inconsistency] docs/plans/gitlab-support.md — Duplicate step numbering in the GitLabCredentialBackend.MintToken procedure: steps are numbered 1, 2, 3, 3, 4, 5 (two step 3s). The first step 3 describes decomposing the project path; the second describes retrieving the PAT from Secret Manager.

  • [pattern-inconsistency] docs/problems/gitlab-implementation.md — This file retains many hyphenated ADR references (ADR-0028, ADR-0009, ADR-0005) outside the lines modified by this PR, while the PR's own changes and all other modified files use the space-separated format (ADR 0028). The file is now marked superseded, so this is low priority cleanup.

  • [internal-inconsistency] docs/plans/gitlab-support.md — The fullsend admin analyze behavior for CI_DEBUG_TRACE is described as "blocks the analysis report" while fullsend admin install uses "hard error." It is ambiguous whether "blocks the analysis report" means immediate non-zero exit (like install) or whether analyze continues checking other conditions but withholds output.

Info

  • [design-observation] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md — The WEBHOOK_VALIDATED=true flag trust boundary and the CI_DEBUG_TRACE guard limitation are both explicitly acknowledged in the document with thorough rationale and mitigations. The design trade-offs are sound and well-documented.

  • [design-observation] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md — The PAT scope regression (no scope-down at mint time, up to 90-day exposure window vs GitHub's 1-hour TTL) is explicitly documented as an accepted trade-off with mitigations (rotation automation, per-role scoping, Secret Manager storage).

  • [scope-alignment] The PR correctly implements all deliverables from issue GitLab support via webhook bridge (ADR 0043) #1964: ADR 0043, companion implementation plan, ADR 0028 status update, and cross-reference updates across the documentation.

Previous run (8)

Review

Findings

The prior medium-severity finding (RefProtected comment contradicting GitHub behavior) has been resolved — the comment now correctly scopes enforcement to GitLab-only. One new medium-severity finding: the Security Considerations section and ADR 0043 both incorrectly attribute webhook secret validation to the dispatch pipeline.

Medium

  • [technical-accuracy] docs/plans/gitlab-support.md — The Security Considerations section states the webhook secret is "Validated by the dispatch pipeline using constant-time comparison." This is inaccurate: the constant-time comparison is performed by the bridge Cloud Function (validateWebhookToken in internal/bridge/main.go). The dispatch pipeline only checks the boolean WEBHOOK_VALIDATED==true flag set by the bridge after its validation succeeds. The same inaccuracy appears in ADR 0043's defense-in-depth section: "The dispatch pipeline validates the token before processing." This could mislead implementers about which component is responsible for the cryptographic validation. See also: [edge-case] WEBHOOK_VALIDATED bypass finding below.
    Remediation: In docs/plans/gitlab-support.md Security Considerations, change "Validated by the dispatch pipeline using constant-time comparison" to "Validated by the bridge Cloud Function using constant-time comparison before triggering the dispatch pipeline." In ADR 0043, change "The dispatch pipeline validates the token before processing" to "The bridge Cloud Function validates the token (constant-time comparison) before triggering the dispatch pipeline."

Low

  • [internal-consistency] docs/plans/gitlab-support.md — The NormalizedClaims.RefProtected field comment states "The mint rejects GitLab requests where RefProtected is false," and the field is correctly populated for GitLab only (with a clear note that GitHub uses a separate validation path). However, the enforcement pseudocode is absent — no snippet shows the actual rejection logic in the mint's request handler. An implementer relying solely on this plan could omit the check. Consider adding a brief enforcement snippet (e.g., if forge == ForgeGitLab && !claims.RefProtected { return error }).

  • [credential-exposure] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md — PAT exposure window (up to 90 days with enforced 120-day creation expiry and 90-day rotation cadence) is a documented security regression vs GitHub's 1-hour tokens. The ADR is transparent about this trade-off with multiple mitigations (automated rotation via default scaffold scheduled pipeline, per-role scoping, Secret Manager storage, audit log alerting recommendation). Consider elevating audit log alerting from "recommended" to a required install-time configuration. Anchored from prior review.

  • [edge-case] docs/plans/gitlab-support.md — The WEBHOOK_VALIDATED=true flag can be set by anyone with the trigger token, bypassing per-project webhook secret validation. The document extensively documents this trust boundary (bridge code comments and dispatch pipeline comments both explain that trigger token possession equates to Maintainer trust). Consider adding a second verification layer (e.g., HMAC-signed payload) to provide independent verification beyond trigger token possession. Anchored from prior review.

  • [edge-case] docs/plans/gitlab-support.md — The CI_DEBUG_TRACE in-script guard fires after GitLab has already printed secrets to logs. The plan correctly treats this as a hard error during install and in fullsend admin analyze, with layered mitigations (install-time check, runtime guard, analyze-time check). Inherent GitLab platform limitation. Anchored from prior review.

  • [edge-case] docs/plans/gitlab-support.mdGitLabCredentialBackend.MintToken decomposes repos[0] by splitting on / to extract group and project components. If a project path has no / (e.g., a top-level namespace), the group would be empty and the Secret Manager key format would be malformed. Add validation: if the project path has no /, return an error.

  • [path-traversal] docs/plans/gitlab-support.md — The /_ escaping for Secret Manager naming is not injective (myorg_subgroup and myorg/subgroup collide). The install flow includes a collision check at enrollment time, but concurrent enrollments by different operators could race. Documented as an accepted edge case.

  • [architectural-coherence] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md — The mint's role shifts from generating short-lived, scope-limited tokens (GitHub) to storing and returning pre-created, long-lived PATs (GitLab). The ADR explicitly acknowledges this in its Open Questions section with a concrete alternative (group-level bot PAT generation) and decision criteria for revisiting. Anchored from prior review.

  • [architectural-coherence] docs/ADRs/0043-gitlab-support-via-webhook-bridge.md — ADR 0043 introduces a hosted webhook bridge, departing from ADR 0009's rejection of hosted webhook receivers. The ADR addresses this tension directly with three justifications: precedent (ADR 0029 mint), necessity (no pull_request_target equivalent), and scope (GitLab-only). Anchored from prior review.

  • [pattern-inconsistency] docs/ADRs/0036-agent-execution-sandbox.md — The References section is normalized to space format (ADR 0005, ADR 0017) but the body text retains hyphenated format in sections not touched by this PR, creating intra-file inconsistency. Pre-existing issue made more visible by targeted updates. Anchored from prior review.

  • [pattern-inconsistency] docs/plans/agent-execution-environment.md — The References section is normalized to space format but the body text retains hyphenated format (ADR-0036, ADR-0023, ADR-0030), creating intra-file inconsistency. Pre-existing issue made more visible by targeted updates. Anchored from prior review.

  • [pattern-inconsistency] docs/problems/gitlab-implementation.md — The References section partially normalizes to space format while the body text retains hyphenated format. Since the file is now marked superseded, full normalization is low priority. Anchored from prior review.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 2, 2026
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 2, 2026
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 3, 2026
Comment thread docs/plans/gitlab-support.md
Comment thread docs/problems/gitlab-support.md Outdated
Comment thread docs/plans/gitlab-support.md Outdated
Comment thread docs/plans/gitlab-support.md
Comment thread docs/plans/gitlab-support.md
rh-hemartin

This comment was marked as off-topic.

@rh-hemartin
rh-hemartin self-requested a review June 3, 2026 06:34
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md Outdated
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md Outdated
@ggallen
ggallen force-pushed the docs/adr-0043-gitlab-support branch from 3ca0501 to 130d954 Compare June 3, 2026 14:37
@ggallen
ggallen force-pushed the docs/adr-0043-gitlab-support branch from 130d954 to de8ee02 Compare June 3, 2026 14:40
@ggallen
ggallen requested review from ifireball and maruiz93 June 3, 2026 14:41
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 3, 2026
@ggallen
ggallen force-pushed the docs/adr-0043-gitlab-support branch from de8ee02 to 0fa7e2e Compare June 3, 2026 15:37
@ggallen

ggallen commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

All findings from the latest review addressed in the squashed commit:

Medium — fixed:

  • [logic-error] MinimalPayload struct — Added Changes field with nested MinimalChanges and MinimalLabelChanges structs that capture changes.labels.current and changes.labels.previous. The determine-stage script's jq expression for label diffing now has the data it needs.

Low — fixed:

  • [edge-case] getAudience — Now accepts an expected audience parameter and iterates over all elements in the aud array to check if the expected audience (fullsend-mint) is present, rather than blindly returning aud[0]. Falls back to first element if the expected audience isn't found.
  • [scope-coherence] Alternative 3 ambiguity — Removed the sentence in the Credential model section that re-opened Alternative 3 as a "viable alternative." The mint statefulness trade-off paragraph now clearly accepts the per-project storage cost as worthwhile for the centralized credential management benefits.
  • [architectural-coherence] Alternative 4 criteria — Replaced the open-ended "warrants a proof-of-concept" with concrete decision criteria: recommended for multiple self-hosted instances behind VPNs, not recommended for GitLab.com or single-instance deployments. Clarified it may be dropped if the webhook bridge proves viable.
  • [architectural-coherence] PAT blast radius — Added a risk entry documenting that compromised PATs grant persistent role-level access until rotation (up to 1 year), with mitigations: max expiry enforcement, automated rotation, Secret Manager storage, and per-role scoping.

Info — fixed:

  • [cross-reference-style] Removed redundant "(Deprecated)" label from the supersession note — ADR 0028's status is tracked in its own frontmatter.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 3, 2026
@Dharit13

This comment was marked as off-topic.

@ggallen
ggallen force-pushed the docs/adr-0043-gitlab-support branch from 0fa7e2e to 9039e05 Compare June 3, 2026 16:31
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 5, 2026
@ggallen

ggallen commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

Addressing review findings:

Unrelated skill changes: Removed cutting-releases skill changes that slipped in.

docs/problems/ misuse: Moved docs/problems/gitlab-support.mddocs/plans/gitlab-support.md since it's implementation-specific, not a general SDLC problem. Updated all references in README.md, roadmap.md, and ADR 0043.

Local webhook server approach: Yes, exactly. The feat/dev-mint-v2 base branch added a local mint development server (cmd/devmint) to test mint flows locally without deploying to GCP. The GitLab webhook bridge will follow the same pattern:

  • Development: Local server under cmd/devwebhook (or similar) for testing webhook translation without Cloud Function deployment
  • Production: Graduate to Cloud Run/Cloud Function deployment (same as the mint graduated from devmint → GCF)

This keeps the development workflow consistent and avoids requiring GCP deployments for local testing. The production webhook bridge will live as a standalone command under cmd/ as you suggested, with no shared code under internal/ since it's a separate service (just like the mint function).

Squashing and force-pushing.

@ggallen

ggallen commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

Rebased onto clean main to remove unrelated changes (mint code, scaffold updates, skill changes). PR now contains only the ADR 0043 documentation:

  • New ADR 0043: GitLab support via webhook bridge
  • Companion implementation plan in docs/plans/gitlab-support.md
  • ADR 0028 marked as superseded
  • Cross-references updated in related ADRs and docs
  • README and roadmap updated

All changes are docs-only. Ready for final review.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Jun 5, 2026
@ggallen

ggallen commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

Resolved all fullsend-ai-review bot findings:

Medium:

  • [internal-consistency] Resource group inconsistency fixed: Bridge now extracts ISSUE_IID and MR_IID from webhook payloads and passes them as pipeline variables. Updated triage.yml example to use fullsend-triage-${SOURCE_PROJECT}-${ISSUE_IID} for per-issue concurrency matching GitHub's behavior.

Low:

  • [interface-design] Multi-repo limitation documented: GitLabCredentialBackend.MintToken now validates len(repos) == 1 and returns error for multi-repo requests (GitLab PATs are per-project, unlike GitHub installation tokens).
  • [adr-reference-format] ADR reference format fixed: Updated ADR-0028 and ADR-0043 references in 0036-agent-execution-sandbox.md to use hyphenated format (ADR-0028, ADR-0043) consistently with the file's existing convention.
  • [oidc-audience-handling] getAudience note added: Comment in gitlab-support.md notes this function should also be used for GitHub token validation in Phase 3 to ensure consistent audience handling across both forges.

Info:

  • ℹ️ [webhook-token-validation-timing] Acknowledged - already documented as not practically exploitable.

All findings resolved and pushed.

Comment thread docs/ADRs/0036-agent-execution-sandbox.md
Comment thread docs/plans/gitlab-support.md
Comment thread docs/plans/gitlab-support.md Outdated
Comment thread docs/plans/gitlab-support.md

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread docs/plans/gitlab-support.md
Comment thread docs/plans/gitlab-support.md
Comment thread docs/plans/gitlab-support.md
Comment thread docs/problems/gitlab-implementation.md
Comment thread docs/plans/gitlab-support.md
Comment thread docs/problems/gitlab-implementation.md Outdated
Comment thread docs/plans/gitlab-support.md
Comment thread docs/problems/gitlab-implementation.md
Comment thread docs/plans/agent-execution-environment.md
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread docs/plans/gitlab-support.md
Comment thread docs/ADRs/0043-gitlab-support-via-webhook-bridge.md
Comment thread docs/plans/gitlab-support.md
Comment thread docs/plans/gitlab-support.md
Comment thread docs/plans/gitlab-support.md
Adds ADR 0043 documenting the GitLab support architecture via webhook
bridge Cloud Function. Supersedes ADR 0028.

Key decisions:
- Webhook bridge Cloud Function translates GitLab webhooks to pipeline triggers
- Central token mint extended with GitLab OIDC and Project Access Tokens
- Protected branch enforcement via hardcoded ref=main in bridge
- Per-project webhook secrets for validation
- GitLab CI/CD templates mirror GitHub workflows structure

Related changes:
- Add companion implementation plan at docs/plans/gitlab-support.md
- Mark ADR 0028 as superseded
- Update cross-references in ADRs 0007, 0031, 0036
- Update roadmap with current GitLab support status

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
**Concern raised during review**: The mint was designed to *generate* short-lived, scope-limited tokens (GitHub App installation tokens). For GitLab, it stores and returns pre-created PATs — it does not actually mint anything. This raises two issues: (1) the mint no longer provides scope-limiting, which was a core security guarantee; (2) each project enrollment requires writing a PAT into the shared mint's Secret Manager, which complicates the "public mint" goal of zero-touch onboarding.

**Current rationale**: The mint still provides OIDC claim validation (credentials only reach pipelines running `.fullsend` on a protected ref) and centralized audit logging. Without it, PATs would need to be stored as protected CI/CD variables in `.fullsend`, visible to all Maintainers. The mint-via-OIDC path avoids persisting credentials in CI/CD variables.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the PATs are per-project, it does not seem to make sense to store them in .fullsend - instead each project would have its own variables, scoped for accessing that project.

This does raise another question - if PATs are project-scoped, how do we do cross-project operations? Do we need to copy the PATs of one project into the secrets of the project that is accessing it?

@waynesun09

Copy link
Copy Markdown
Member

Missing alternative: GitLab include: project: ref: as native pull_request_target equivalent

The ADR's core argument for the webhook bridge is:

GitLab has no equivalent mechanism [to pull_request_target]. A .gitlab-ci.yml in an enrolled repo runs the MR branch version — an attacker could modify it to dump secrets.

This is true for a raw in-repo .gitlab-ci.yml (Alternative 1). But the ADR doesn't evaluate GitLab's cross-project include: mechanism, which provides the same security guarantee:

# In enrolled repo's .gitlab-ci.yml
include:
  - project: 'org/.fullsend'
    file: 'templates/dispatch.yml'
    ref: 'main'  # pulled from protected branch of the .fullsend project

With include: project: ref: 'main':

  1. Template code is pulled from the .fullsend project's protected main branch — the MR author cannot modify it, analogous to how pull_request_target runs workflow code from the base branch.
  2. The enrolled repo's .gitlab-ci.yml can be modified by the MR author — but the ADR's own defense-in-depth layer (protected CI/CD variables only exposed on protected branches) already prevents secret exfiltration from MR branch pipelines.
  3. merge_request_event and push pipeline sources trigger natively — no JSON-to-form-encoded translation, no wire protocol mismatch, no intermediary needed.

Security comparison

Security layer Webhook bridge (proposed) include: project: ref: main
Agent code runs from protected ref Bridge hardcodes ref=main Template pulled from .fullsend main
Secrets hidden from MR branches Protected CI/CD variables Protected CI/CD variables (same mechanism)
Per-project auth validation Webhook secret in Secret Manager Not needed — no inbound webhooks
Infrastructure to deploy/monitor Cloud Function per GitLab instance None — GitLab CI runner handles it

What this eliminates

  • The bridge Cloud Function (and per-instance deployment for self-hosted GitLab)
  • Per-project webhook secret registration and Secret Manager storage
  • Webhook secret validation logic
  • Bridge availability monitoring
  • The wire protocol translation layer

What still needs solving

The include: pattern handles MR events (merge_request_event) and push events natively. It does not cover issue events, comment/note events, or label events — GitLab CI has no native pipeline source for those.

For those events, the ADR's deferred Alternative 4 (scheduled pipeline polling) or a narrowly scoped webhook-to-trigger proxy would be needed. But that's a much smaller problem than a full webhook bridge for all event types — MR/push events are the high-frequency, security-sensitive path.

Suggestion

Consider adding include: project: ref: main as Alternative 5 in the ADR and evaluating it against the webhook bridge. If accepted, the bridge could be reduced to only handling event types that GitLab CI can't trigger natively (issue/comment events), rather than being the universal dispatch mechanism for all GitLab events.

@ggallen

ggallen commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

Superseeded by #2042.

@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:01 PM UTC · Completed 12:08 PM UTC
Commit: ba204cb · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1816 — ADR 0043 GitLab webhook bridge

This human-authored PR (by ggallen, co-authored with Claude Sonnet 4.5) added ADR 0043 and a companion implementation plan for GitLab support via a webhook bridge. It was open for 7 days, received substantive architectural feedback from 4 human reviewers, went through 25 force-pushes and 9+ review bot runs, and was ultimately closed without merge — superseded by PR #2042 after ifireball and waynesun09 raised fundamental concerns about the mint architecture and proposed alternative approaches.

Key observations

  1. Review bot applied production-code review standards to design documents. The implementation plan (docs/plans/gitlab-support.md) contained illustrative code examples (Go snippets, YAML pipelines, CLI commands). The review bot flagged bugs in these examples — e.g., normalizeClaims type assertion errors, missing CI_DEBUG_TRACE guards — treating pseudocode as production code. While technically accurate, these findings were noise on a design document that existed to communicate intent, not ship code.

  2. Most improvement areas are already tracked. The existing issue corpus extensively covers: redundant re-reviews on force-pushes (Cancel-and-skip redundant re-reviews on force-pushed PRs with no prior findings #1372, Deduplicate review runs on rapid successive pushes #1418, Deduplicate review runs when PR is rebased multiple times in quick succession #1422), label oscillation (Consider gating ready-for-merge label on human approval status #1574, Filter bot-triggered label events to reduce duplicate workflow runs #1362), docs/ADR review behavior (Review agent should defer ready-for-merge label on documentation-only PRs until human review #1772, Review agent should comment-only (not approve) on ADR PRs #1659, Review agent should not suggest adding implementation details to ADRs #1660), review-fix circuit breakers (Add circuit breaker to review-fix feedback loop #902), and approach-level review (Review agent should evaluate approach soundness, not just implementation correctness #1999, recently closed). No new proposals are warranted for these patterns.

  3. The review bot's iteration model mismatched the PR's actual lifecycle. While the bot iterated on implementation-detail findings, humans were debating whether the entire approach was viable. The bot consumed author attention on fixes that became irrelevant when the PR was superseded. However, this is partially addressed by recently-closed Review agent should evaluate approach soundness, not just implementation correctness #1999 (approach soundness review) and open Review agent should check for duplicate/superseding PRs before approving #1313 (check for superseding PRs).

Proposal filed

One proposal below for a genuinely novel gap: the review agent treating code examples in design/planning documents as production code.

Proposals filed

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

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitLab support via webhook bridge (ADR 0043)

6 participants