fix(#6615): auto-add GitLab forge host to sandbox egress allowlist and proxy profile

[fullsend-ai-coder]

Summary

Auto-configures both sandbox network layers for self-hosted GitLab forge hosts, fixing the 403/DNS-failure loop that blocks all agent runs on self-hosted GitLab instances.

Related Issue

Closes #6615

Changes

• Layer 1 (SSRF egress allowlist): appendHookEnv in internal/runtime/sandbox_hooks_bootstrap.go now auto-resolves the GitLab forge hostname from FULLSEND_GITLAB_URL / GITLAB_API_URL / CI_SERVER_URL when GITLAB_CI=true and merges it as host:443 into FULLSEND_EGRESS_ALLOWLIST. This is a tactical bridge until SSRF hook: move DNS resolution to the host API server #6586 moves DNS resolution to the host API server.
• Layer 2 (L7 proxy provider profile): run.go auto-generates a fullsend-gitlab-forge provider profile YAML (modeled after the scaffold's fullsend-github.yaml) with the forge host:443 endpoint and imports it via sandbox.ImportProfile() during the provider/profile setup phase. This is the durable fix — the proxy needs the endpoint regardless of DNS evolution.
• Both layers are gated on GitLab detection and are no-ops for GitHub. URL precedence matches forge_client.go. User-configured ssrf_egress_allowlist entries are preserved (comma-separated merge, deduplication via ParseEgressAllowlist).

Testing

• resolveGitLabForgeHost / resolveGitLabForgeHostFromEnv: table-driven tests covering all URL precedence combinations, non-GitLab no-op, and missing env vars
• appendHookEnv: tests for auto-add on GitLab, merge with user allowlist, no-op when not on GitLab, URL precedence
• generateGitLabForgeProfile: tests for success (validates YAML content), no-host returns empty, URL precedence
• All existing tests in internal/runtime/ and internal/cli/ continue to pass (2 pre-existing failures in TestPostScriptEnv_NoSchema* unrelated to this change)

---

Closes #6615

Post-script verification

• Branch is not main/master (agent/6615-gitlab-forge-egress)
• Secret scan passed (gitleaks — bfa5a2afa6f0e1ae46c6d7d7d142aded36372663..HEAD)
• PR body secret scan passed (gitleaks — no-git)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:08 AM UTC · Completed 1:29 AM UTC

Commit: 1be2315 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.04

[codecov]

Codecov Report

❌ Patch coverage is 81.15942% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/gitlab_profile.go	64.51%	6 Missing and 5 partials ⚠️
internal/forge/gitlab/env.go	87.50%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Moderate risk: bot-authored, well-scoped GitLab sandbox networking fix with solid test coverage (36%). File and line counts are elevated (14 files, 496 lines) but most additions are tests and new gitlab_profile.go/env.go modules. run.go remains an extreme churn hotspot (84 commits/30d, 17 authors) warranting careful review. Security-adjacent but additive and gated on forgePlatform==gitlab. Change is platform-gated and easily revertible. Prior score of 2 confirmed — signals unchanged.

Previous run

Risk Assessment: moderate (2/5)

Details

Moderate risk: bot-authored, well-scoped GitLab sandbox networking fix with solid test coverage (38%). File and line counts are elevated (13 files, 470 lines) but most additions are tests and a new gitlab/env.go module. run.go remains an extreme churn hotspot (84 commits/30d, 14 authors) warranting careful review. Security-adjacent (sandbox egress/proxy) but additive and gated on forgePlatform==gitlab with no impact to the GitHub path. Issue scope aligns well with PR changes. Prior score of 2 confirmed — no new signals warrant adjustment.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Moderate risk: bot-authored, well-scoped GitLab sandbox networking fix with solid test coverage (38%). File and line counts are elevated (13 files, 474 lines) but most additions are tests and a new gitlab/env.go module. run.go remains an extreme churn hotspot warranting careful review. Security-adjacent (sandbox egress/proxy) but additive and gated on forgePlatform==gitlab with no impact to the GitHub path.

Previous run (3)

Risk Assessment: moderate (2/5)

Details

Moderate risk: well-scoped bot-authored bug fix with good test coverage (31%), but run.go is an extremely high-churn file (323 commits, 84 in last 30 days) warranting careful review. The change is security-adjacent (sandbox egress allowlist) but narrowly scoped to adding GitLab forge hosts.

Previous run (4)

Risk Assessment: moderate (2/5)

Details

Moderate risk: well-scoped bot-authored bug fix with good test coverage (36%), but run.go is an extremely high-churn file warranting careful review.

Previous run (5)

Risk Assessment: moderate (2/5)

Details

Moderate risk: 8 files with 430 lines changed (mostly additive), good test coverage ratio (0.38), bot author, no protected paths or CI workflow changes. The security label and high priority on the linked issue elevate concern, but the change is narrowly scoped to sandbox egress networking for GitLab.

Previous run (6)

Risk Assessment: moderate (2/5)

Details

Medium-sized bot-authored bug fix (417 lines, 8 files) with good test coverage (38% test files). Two production files (run.go, customizing-agents.md) are high-churn hotspots, but the change is well-scoped, additive, and aligns precisely with the linked issue.

Previous run (7)

Risk Assessment: moderate (2/5)

Details

Well-scoped bot PR with strong test coverage (50% test files) and low security/CI impact, but touches an extremely high-churn file (run.go) which elevates git-history risk.

[fullsend-ai-review]

Review

Findings

Low

• [edge-case] internal/forge/gitlab/env.go:11 — ResolveForgeHostPort covers the env-var portion of the precedence chain (FULLSEND_GITLAB_URL > GITLAB_API_URL > CI_SERVER_URL), which matches forge_client.go. The manifest-level override (repos.Manifest.GitLab.URL) is excluded by design since it is a per-call API client parameter, not an environment variable available in the env-only resolution context. The refactoring in forge_client.go to use the shared URLEnvVars slice is semantically identical to the prior hardcoded cascade.

• [architectural-coherence] internal/cli/run.go — gl.ResolveForgeHostPort() is called twice when forgePlatform is "gitlab": once around line 1098 (via generateGitLabForgeProfile()) and once around line 1554 (egress entry). Both calls are deterministic env-var reads and the inline comment at line 1553 documents this explicitly. The two call sites serve distinct consumers (L7 proxy profile vs. SSRF allowlist) at different phases.

• [ssrf-allowlist-expansion] internal/runtime/sandbox_hooks_bootstrap.go:88 — Security analysis: the forge egress entry auto-merge correctly appends to the existing SSRF egress allowlist. The allowlist only bypasses the DNS-resolution fail-closed gate — BLOCKED_HOSTNAMES and BLOCKED_NETWORKS checks remain unconditionally enforced. The entry derives from operator/runner-controlled env vars, not PR-author-controlled input. No privilege escalation or SSRF bypass identified.

• [input-validation] internal/forge/gitlab/env.go:18 — Security analysis: url.Parse with hostname-emptiness guard provides safe parsing. url.Hostname() strips IPv6 brackets; the hostname flows into YAML via Go struct marshaling (injection-safe) and into the shell env var via appendEnvVar's single-quote escaping (injection-safe). The port is from url.Port() (digits only per RFC) or hardcoded "80"/"443".

• [fail-open-analysis] internal/cli/run.go:1098 — Security analysis: all failure modes result in skip/degrade — no entry is added to the allowlist, no profile is generated. generateGitLabForgeProfile errors are logged as warnings. ResolveForgeHostPort returns empty on invalid URLs. The entire feature is gated on forgePlatform == "gitlab". When security is disabled, newHarnessBootstrap returns base without hooks. No path results in elevated access on failure.

• [comment-style] internal/cli/run.go:1555 — The inline NOTE: comment at the forgeEgressEntry resolution block cross-references the dual gl.ResolveForgeHostPort() call. In a 4900+ line file this serves locality-of-understanding; minor staleness risk if either call site changes.

• [doc-comment-consistency] internal/security/sandbox_hooks.go:63 — WithForgeEgressEntry doc comment describes the contract abstractly ("merged into the egress allowlist at bootstrap time") while appendHookEnv's doc comment carries the implementation detail. Both are individually accurate and appropriately scoped.

Previous run

Review

Findings

Low

• [edge-case] internal/forge/gitlab/env.go:11 — ResolveForgeHostPort covers the env-var portion of the precedence chain (FULLSEND_GITLAB_URL > GITLAB_API_URL > CI_SERVER_URL), which matches forge_client.go. The manifest-level override (repos.Manifest.GitLab.URL) is excluded by design since it is a per-call API client parameter, not an environment variable available in the env-only resolution context.

• [architectural-coherence] internal/cli/run.go — gl.ResolveForgeHostPort() is called twice when forgePlatform is "gitlab": once around line 1098 (via generateGitLabForgeProfile()) and once around line 1554 (egress entry). Both calls are deterministic env-var reads and the inline comment at line 1553 documents this explicitly.

• [ssrf-allowlist-expansion] internal/runtime/sandbox_hooks_bootstrap.go:88 — Security analysis: the forge egress entry auto-merge correctly appends to the existing SSRF egress allowlist. The allowlist only bypasses the DNS-resolution fail-closed gate — BLOCKED_HOSTNAMES and BLOCKED_NETWORKS checks remain unconditionally enforced. The entry derives from operator/runner-controlled env vars, not PR-author-controlled input. No privilege escalation or SSRF bypass identified.

• [input-validation] internal/forge/gitlab/env.go:21 — Security analysis: url.Parse with hostname-emptiness guard provides safe parsing. url.Hostname() strips IPv6 brackets; the hostname flows into YAML via Go struct marshaling (injection-safe) and into the shell env var via appendEnvVar's single-quote escaping (injection-safe). The port is from url.Port() (digits only per RFC) or hardcoded "80"/"443".

• [fail-open-analysis] internal/cli/run.go:1092 — Security analysis: all failure modes result in skip/degrade — no entry is added to the allowlist, no profile is generated. No path results in elevated access on failure.

• [comment-style] internal/cli/run.go:1555 — The inline NOTE: comment at the forgeEgressEntry resolution block cross-references the dual gl.ResolveForgeHostPort() call. In a 4900+ line file this serves locality-of-understanding; minor staleness risk if either call site changes.

• [doc-comment-consistency] internal/security/sandbox_hooks.go:63 — WithForgeEgressEntry doc comment describes the contract abstractly ("merged into the egress allowlist at bootstrap time") while appendHookEnv's doc comment carries the implementation detail. Both are individually accurate and appropriately scoped.

• [type-placement] internal/cli/run.go:4100 — gitlabForgeEndpoint, gitlabForgeProfileSpec, and generateGitLabForgeProfile are defined at the bottom of the ~4900-line run.go. The codebase has dedicated files for forge-specific helpers (e.g., forge_client.go). Consider extracting to internal/cli/gitlab_profile.go in a follow-up.
  Remediation: Extract to internal/cli/gitlab_profile.go.

Previous run (2)

Review

Findings

Low

• [error-handling] internal/cli/run.go:4157 — yaml.Encoder.Close() returns an error that is silently discarded. While writing to a bytes.Buffer makes a Close failure extremely unlikely, this is a minor gap in error handling for defense-in-depth.
  Remediation: Check the error: if err := enc.Close(); err != nil { return "", nil, fmt.Errorf("closing YAML encoder: %w", err) }

• [comment-style] internal/cli/run.go:4130 — generateGitLabForgeProfile doc comment includes a NOTE: paragraph about gl.ResolveForgeHostPort() being called elsewhere. The same note also appears as an inline comment at the call site (lines 1549–1552). Duplicated cross-reference comments are a staleness risk.
  Remediation: Keep the cross-reference at the call site only; remove the NOTE: from the function doc comment.

• [doc-comment-consistency] internal/security/sandbox_hooks.go:63 — WithForgeEgressEntry doc comment references appendHookEnv (a specific consumer in the runtime package), coupling the doc to an implementation detail. Other SandboxHookConfig methods describe what the value is, not who reads it.
  Remediation: Simplify to describe the contract: "The entry is a host:port string merged into the egress allowlist at bootstrap time."

• [ssrf-allowlist-expansion] internal/runtime/sandbox_hooks_bootstrap.go:88 — Security analysis: the forge egress entry auto-merge correctly appends to the existing SSRF egress allowlist. The allowlist only bypasses the DNS-resolution fail-closed gate — BLOCKED_HOSTNAMES and BLOCKED_NETWORKS checks remain unconditionally enforced. No privilege escalation or SSRF bypass identified.

• [input-validation] internal/forge/gitlab/env.go:11 — Security analysis: url.Parse with hostname-emptiness guard provides safe parsing. The hostname flows into YAML via Go struct marshaling (injection-safe) and shell env vars via single-quote escaping in appendEnvVar (injection-safe). The env var sources (CI_SERVER_URL, FULLSEND_GITLAB_URL) are operator/runner-controlled, not PR-author-controlled. No validation gap identified.

• [fail-open-analysis] internal/cli/run.go:1092 — Security analysis: all failure modes (empty env vars, URL parse errors, empty hostname, strconv.Atoi failure) result in skip/degrade — no entry is added to the allowlist, no profile is generated. No path results in elevated access on failure.

• [edge-case] internal/forge/gitlab/env.go:11 — ResolveForgeHostPort covers the env-var portion of the precedence chain (FULLSEND_GITLAB_URL > GITLAB_API_URL > CI_SERVER_URL), which matches forge_client.go. The manifest-level override (repos.Manifest.GitLab.URL) is excluded by design since it is a per-call API client parameter, not an environment variable available in the env-only resolution context.

• [architectural-coherence] internal/cli/run.go — gl.ResolveForgeHostPort() is called twice when forgePlatform is "gitlab": once around line 1549 (egress entry) and once inside generateGitLabForgeProfile() (profile). Both calls read only environment variables and are deterministic. The in-code comment cross-references the two call sites.

Previous run (3)

Review

Findings

Low

• [scope-consistency] internal/cli/run.go:1549 — gl.ResolveForgeHostPort() is called twice when forgePlatform is "gitlab": once around line 1549 (egress entry) and once inside generateGitLabForgeProfile() (profile). Both calls read only environment variables and are deterministic. The in-code comment at the egress call site now cross-references the profile call, improving discoverability compared to the prior review.

• [edge-case] internal/forge/gitlab/env.go:11 — ResolveForgeHostPort covers the env-var portion of the precedence chain (FULLSEND_GITLAB_URL > GITLAB_API_URL > CI_SERVER_URL), which matches forge_client.go. The manifest-level override (repos.Manifest.GitLab.URL) is excluded by design since it is a per-call API client parameter, not an environment variable available in the env-only resolution context.

• [ssrf-allowlist-expansion] internal/runtime/sandbox_hooks_bootstrap.go:91 — Security analysis: the auto-merge of the forge egress entry is correctly gated by ForgeEgressEntry() (set by the CLI layer via SandboxHookConfig.WithForgeEgressEntry()). The allowlist only bypasses the DNS-resolution fail-closed gate — BLOCKED_HOSTNAMES and BLOCKED_NETWORKS checks remain enforced unconditionally. No privilege escalation or SSRF bypass identified.

• [input-validation] internal/forge/gitlab/env.go:25 — Security analysis: url.Parse with hostname emptiness guard provides safe parsing. The hostname flows into YAML via Go struct marshaling (injection-safe) and shell env vars via single-quote escaping in appendEnvVar (injection-safe). The env var sources (CI_SERVER_URL, FULLSEND_GITLAB_URL) are operator/runner-controlled, not PR-author-controlled. No validation gap identified.

• [fail-open-analysis] internal/runtime/sandbox_hooks_bootstrap.go:88 — Security analysis: all failure modes (empty env vars, URL parse errors, empty hostname, strconv.Atoi failure) result in skip/degrade — no entry is added to the allowlist, no profile is generated. No path results in elevated access on failure.

• [provider-profile-generation] internal/cli/run.go:1105 — Security analysis: the auto-generated GitLab forge profile is prepended to the profiles list so that a user-defined profile with the same ID (fullsend-gitlab-forge) wins via last-wins dedup. The temp file is cleaned up via defer. Correct precedence — no security concern.

Info

• [refactor-correctness] internal/cli/forge_client.go:49 — The env-var precedence chain for GitLab base URL was refactored from three inline if-else branches to a loop over the shared gl.URLEnvVars slice. The refactor introduces a single source of truth for env-var precedence, eliminating a precedence-drift risk. No security regression. See also: prior [design-fit] finding at this location (resolved by this change).

• [naming] internal/cli/run.go:4072 — The type gitlabForgeProfileSpec uses the suffix Spec which does not appear elsewhere in run.go — the existing pattern uses no suffix or plain nouns. Consider renaming to gitlabForgeProfile for consistency.
  Remediation: Rename gitlabForgeProfileSpec to gitlabForgeProfile.

• [doc-comment] internal/forge/gitlab/env.go:11 — The doc comments on both URLEnvVars and ResolveForgeHostPort say "Precedence matches forge_client.go," but forge_client.go now derives its precedence from gl.URLEnvVars itself. The comments are circularly referential. Since URLEnvVars is now the canonical source of truth, its doc comment should state the precedence directly.
  Remediation: Update both doc comments in env.go to state the precedence order directly and remove the "matches forge_client.go" references.

Previous run (4)

Review

Findings

Low

• [design-fit] internal/cli/forge_client.go:49 — forge_client.go retains its own inline env-var precedence chain (FULLSEND_GITLAB_URL > GITLAB_API_URL > CI_SERVER_URL) duplicating logic now centralized in gitlab.URLEnvVars / ResolveForgeHostPort(). The two call sites have different requirements (forge_client.go also handles the manifest base URL), but the env-var precedence logic is identical and could diverge silently if updated in one location but not the other.
  Remediation: Consider having forge_client.go delegate to gl.URLEnvVars or a shared helper for the env-var precedence chain.

• [incomplete-documentation] docs/guides/dev/cli-internals.md:384 — The sandbox lifecycle diagram shows the step-by-step flow for run.go but doesn't reflect the new auto-generated GitLab forge profile step inserted before profile import. Minor gap since the diagram already omits several intermediate steps (e.g., dedupResolvedProviders, profile integrity checks).
  Remediation: Consider adding a note between profile dedup and ImportProfile indicating that on GitLab, a fullsend-gitlab-forge profile is auto-generated from the forge host URL.

• [scope-consistency] internal/cli/run.go:1549 — gl.ResolveForgeHostPort() is called twice when forgePlatform is "gitlab": once at line 1549 (egress entry) and once at line 4096 inside generateGitLabForgeProfile() (profile). Both calls read only environment variables and are deterministic. Not a defect, but a cross-referencing comment at either call site would improve discoverability.

• [edge-case] internal/forge/gitlab/env.go:11 — ResolveForgeHostPort covers the env-var portion of the precedence chain (FULLSEND_GITLAB_URL > GITLAB_API_URL > CI_SERVER_URL), which matches forge_client.go. The manifest-level override (repos.Manifest.GitLab.URL) is excluded by design since it is a per-call API client parameter, not an environment variable available in the env-only resolution context.

• [ssrf-allowlist-expansion] internal/runtime/sandbox_hooks_bootstrap.go:91 — Security analysis: the auto-merge of the forge egress entry is correctly gated by ForgeEgressEntry() (set by the CLI layer via SandboxHookConfig.WithForgeEgressEntry()). The allowlist only bypasses the DNS-resolution fail-closed gate — BLOCKED_HOSTNAMES and BLOCKED_NETWORKS checks remain enforced unconditionally. No privilege escalation or SSRF bypass identified.

• [input-validation] internal/forge/gitlab/env.go:25 — Security analysis: url.Parse with hostname emptiness guard provides safe parsing. The hostname flows into YAML via Go struct marshaling (injection-safe) and shell env vars via single-quote escaping in appendEnvVar (injection-safe). The env var sources (CI_SERVER_URL, FULLSEND_GITLAB_URL) are operator/runner-controlled, not PR-author-controlled. No validation gap identified.

• [fail-open-analysis] internal/runtime/sandbox_hooks_bootstrap.go:88 — Security analysis: all failure modes (empty env vars, URL parse errors, empty hostname, strconv.Atoi failure) result in skip/degrade — no entry is added to the allowlist, no profile is generated. No path results in elevated access on failure.

• [provider-profile-generation] internal/cli/run.go:1105 — Security analysis: the auto-generated GitLab forge profile is prepended to the profiles list so that a user-defined profile with the same ID (fullsend-gitlab-forge) wins via last-wins dedup. The temp file is cleaned up via defer. Correct precedence — no security concern.

Previous run (5)

Review

Findings

Medium

• [inconsistent guard condition] internal/runtime/sandbox_hooks_bootstrap.go:88 — The egress allowlist auto-append uses os.Getenv("GITLAB_CI") == "true" as the guard, while the profile auto-generation in run.go uses forgePlatform == "gitlab". These can diverge: detectForgePlatform considers the --forge flag and config.forge in addition to GITLAB_CI. If a user sets config.forge: gitlab without GITLAB_CI=true (e.g., running fullsend against a GitLab repo from a non-GitLab CI runner), the provider profile is generated but the sandbox egress allowlist is not updated, so forge API calls will still fail the SSRF pre-tool hook.
  Remediation: Thread forgePlatform through to appendHookEnv via security.SandboxHookConfig or the SandboxHooksBootstrap interface, so both auto-generation sites use the same resolved platform signal.

Low

• [naming-convention] internal/cli/run.go:4060 — Unexported struct types gitLabForgeEndpoint and gitLabForgeProfileSpec use camelCase gitLab prefix, inconsistent with codebase convention. Existing patterns use all-lowercase: githubSetupConfig, gitlabLabels, gitlabToken.
  Remediation: Rename to gitlabForgeEndpoint and gitlabForgeProfileSpec.

• [code-organization] internal/runtime/sandbox_hooks_bootstrap.go — The runtime package currently has zero dependencies on any forge/* package. Adding the gl import introduces the first forge-specific coupling into the runtime layer. ResolveForgeHostPort reads env vars and parses URLs — not GitLab-client-specific.
  Remediation: Have the CLI layer (which already imports gl) call ResolveForgeHostPort() and pass the resolved host:port into appendHookEnv via a parameter or the existing SandboxHookConfig interface.

• [design-fit] internal/cli/forge_client.go:49 — forge_client.go retains its own inline env-var precedence chain duplicating logic now centralized in gitlab.ResolveForgeHostPort(). Acceptable for a tactical fix but a future maintenance risk if the precedence order changes.

• [ssrf-allowlist-expansion] internal/runtime/sandbox_hooks_bootstrap.go:87 — Security analysis: the auto-append is gated by GITLAB_CI=true (runner-set) and reads from runner-set env vars. The allowlist only bypasses the DNS-resolution fail-closed gate — BLOCKED_HOSTNAMES and BLOCKED_NETWORKS checks remain enforced. No vulnerability identified.

• [input-validation] internal/forge/gitlab/env.go:12 — Security analysis: url.Parse with hostname emptiness guard provides safe parsing. The hostname flows into YAML via struct marshaling and shell env vars via single-quote escaping — both injection-safe. No validation gap identified.

• [fail-open-analysis] internal/runtime/sandbox_hooks_bootstrap.go:88 — Security analysis: all failure modes (empty env vars, parse errors, missing host) result in skip/degrade, never in elevated access. All paths are fail-closed or fail-degraded.

Previous run (6)

Review

Findings

Low

• [injection-vuln] internal/cli/run.go:4070 — The hostname blocklist in generateGitLabForgeProfile covers the characters that can break a YAML double-quoted context (" and \), plus belt-and-suspenders checks for single-quote, newline, and carriage return. The port is validated as a pure integer via strconv.Atoi. The current code is not vulnerable, but the defense relies on knowing which characters are dangerous in YAML double-quoted strings; a YAML serialization library would be more robust against future template changes that might move the value to an unquoted context.
  Remediation: Consider using gopkg.in/yaml.v3 (already imported in run.go) to generate the profile instead of fmt.Sprintf.

• [naming-convention] internal/forge/gitlab/env.go:12 — The exported variable GitLabURLEnvVars uses a stutter pattern — the package is already gitlab, so the qualified name reads gitlab.GitLabURLEnvVars. Other exports in this package (LiveClient, Option, APIError, WithBaseURL) consistently avoid the GitLab prefix.
  Remediation: Rename to URLEnvVars or ForgeURLEnvVars.

Previous run (7)

Review

Findings

Medium

• [edge-case] internal/cli/run.go:4097 — Both resolveGitLabForgeHostFromEnv() and resolveGitLabForgeHost() extract only the hostname via url.Hostname() and hardcode port 443. If the GitLab instance URL includes a non-standard port (e.g. https://gitlab.company.com:8443), the generated provider profile will specify port: 443 and the egress allowlist entry will be gitlab.company.com:443 — neither will match the actual port the forge client uses. This silently defeats the auto-generation for non-443 deployments.
  Remediation: Extract url.Port() in both resolve functions. When non-empty, use the extracted port; otherwise default to "443". Thread the port through to the YAML template’s port: field and the egress allowlist entry.

• [test-inadequate] internal/runtime/sandbox_hooks_bootstrap_test.go:146 — The pre-existing tests TestAppendHookEnv_EgressAllowlist and TestAppendHookEnv_EgressAllowlistEmpty do not clear GITLAB_CI or the GitLab URL environment variables. After this PR, appendHookEnv calls resolveGitLabForgeHost() which reads these env vars. If these tests run in an environment where GITLAB_CI=true and a URL variable is set (e.g., an actual GitLab CI runner), the allowlist assertions will fail.
  Remediation: Add t.Setenv("GITLAB_CI", "") at the top of both tests.

• [code-duplication] internal/cli/run.go:4062 — resolveGitLabForgeHostFromEnv() (run.go) and resolveGitLabForgeHost() (sandbox_hooks_bootstrap.go) contain nearly identical URL-parsing logic with the same env var precedence. Duplicated logic risks divergence if the precedence order is updated in one place but not the other (or in forge_client.go, the source of truth). See also: [naming-convention] finding at this location.

• [stale-doc] docs/guides/user/customizing-agents.md:123 — The ssrf_egress_allowlist field is documented with an empty default showing manual-only configuration. After this PR, the GitLab forge host is auto-appended on GitLab CI, so the field is no longer always empty by default for GitLab users.
  Remediation: Add a note stating that on GitLab CI the forge host is automatically added; manual entries are only needed for additional non-forge hosts.

Low

• [injection-vuln] internal/cli/run.go:4089 — The hostname is interpolated into a YAML template via fmt.Sprintf("%s") without YAML quoting. Go’s url.Hostname() produces clean hostnames that cannot contain YAML-breaking characters, so exploitation is not practical. Adding explicit quoting (host: "%s") would make this defense intentional rather than incidental.

• [inconsistent-gating] internal/cli/run.go:4058 — resolveGitLabForgeHostFromEnv() does not check GITLAB_CI=true before reading environment variables, unlike its counterpart resolveGitLabForgeHost(). The caller already gates on forgePlatform=="gitlab", which is sufficient. See also: [code-duplication] finding.

• [scope-alignment] internal/cli/run.go:1108 — The Layer 2 code path gates on forgePlatform=="gitlab" but not on GITLAB_CI=="true". A local dev running fullsend run --forge gitlab enters this path, but resolveGitLabForgeHostFromEnv() returns empty and the block is a no-op. No functional bug.

• [profile-placement] internal/cli/run.go:1108 — The auto-generated GitLab profile is appended to result.Profiles after checkProviderProfileIntegrity (step 2b) but before the import loop (step 2c), bypassing the integrity check. Acceptable for a system-generated profile with a known-good structure.

• [api-shape] internal/cli/run.go:4078 — The profile YAML is generated via fmt.Sprintf with a raw string template. While run.go imports gopkg.in/yaml.v3, using fmt.Sprintf for a fixed-structure template with a single interpolated hostname is reasonable and readable.

• [naming-convention] internal/cli/run.go:4060 — resolveGitLabForgeHostFromEnv() appends FromEnv but no other resolve* function in run.go uses this suffix. The peer function in sandbox_hooks_bootstrap.go is named resolveGitLabForgeHost() without the suffix.

• [test-naming] internal/cli/run_test.go:3454 — Three TestGenerateGitLabForgeProfile_* functions use individual tests rather than a table-driven test like TestResolveGitLabForgeHostFromEnv. Could be consolidated for consistency.

• [stale-doc] docs/contributing/runtime-implementation.md:100 — The Environment bullet describes appendHookEnv writing FULLSEND_EGRESS_ALLOWLIST but does not mention the new auto-merge of the GitLab forge host into the allowlist on GitLab CI.

• [missing-doc] docs/contributing/runtime-implementation.md:61 — The SSRF pre-tool row in the security feature matrix does not reference the new auto-generated fullsend-gitlab-forge provider profile.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 2:06 AM UTC · Ended 2:25 AM UTC

Commit: a3e9966 · View workflow run →

[github-actions]

Site preview

Preview: https://db26af5e-site.fullsend-ai.workers.dev

Commit: da1827d919db4aec4774f5d7372811804de55114

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:06 AM UTC · Completed 2:25 AM UTC

Commit: a3e9966 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.91

[ggallen]

/fs-fix - rebase and fix all the review issues. Respond to and resolve the conversations.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 2:32 AM UTC · Completed 2:45 AM UTC

Commit: a3e9966 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.14

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Fixed validation failure: the original PR commits contained Signed-off-by trailers which violate the agent commit policy. Squashed both commits (a3e9966 and f4a8ca7) into a single clean commit with only a Co-Authored-By trailer. All prior review fixes (URLEnvVars rename, yaml.v3 marshaling, hostname validation removal, test updates) are preserved in the squashed commit.

Fixed (5):

1. Agent commit contains Signed-off-by trailer (.git): Squashed both PR commits into a single clean commit without any Signed-off-by trailers. The original commit a3e9966 contained duplicate Signed-off-by trailers that violated the agent commit policy. Soft-reset to merge base and recommitted all changes with only a Co-Authored-By trailer.
2. Rename GitLabURLEnvVars to URLEnvVars to avoid stutter (internal/forge/gitlab/env.go): Renamed GitLabURLEnvVars to URLEnvVars so that the qualified name gitlab.URLEnvVars avoids the gitlab.GitLab stutter pattern. Already applied in prior iteration and preserved in the squashed commit.
3. Replace fmt.Sprintf YAML template with yaml.v3 struct marshaling (internal/cli/run.go): Replaced the fmt.Sprintf-based YAML template in generateGitLabForgeProfile with yaml.v3 struct marshaling for structurally safe serialization. Already applied in prior iteration and preserved in the squashed commit.
4. Remove manual hostname character validation (internal/cli/run.go): Removed the manual hostname character validation regex since yaml.Marshal handles quoting automatically. Already applied in prior iteration and preserved in the squashed commit.
5. Update tests to match yaml.v3 output format (internal/cli/run_test.go): Updated test expectations to match yaml.v3 output format (different whitespace and quoting conventions). Already applied in prior iteration and preserved in the squashed commit.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:47 AM UTC · Completed 3:08 AM UTC

Commit: 760f245 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.31

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:25 PM UTC · Completed 5:08 PM UTC

Commit: da1827d · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 6:49 PM UTC · Completed 7:02 PM UTC

Commit: da1827d · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.57

[fullsend-ai-retro]

Retro: PR #6616 — auto-add GitLab forge host to sandbox egress allowlist

Timeline

Time (UTC)	Event
00:41	Issue #6615 opened. Triage agent runs, labels ready-to-code
00:49	Code agent dispatched, creates branch agent/6615-gitlab-forge-egress
01:06	PR #6616 opened (14 files, +479/−17)
01:06–01:29	Review 1: 2 medium + 7 low findings on commit 1be2315. Key: non-standard port bug, code duplication, YAML injection risk
01:39	Human (ggallen) responds inline — "Fixed" on 6 findings, "Acknowledged" on 3
02:30–02:45	Fix 1 (/fs-fix): squashes commits, preserves fixes. Successful
02:47–03:08	Review 3: 1 medium + 3 low. Finds inconsistent gating between egress allowlist and profile generation
03:11–03:26	Fix 2 (/fs-fix): threads forgePlatform through SandboxHookConfig, deduplicates logic. Successful
03:26–03:50	Review 4: APPROVED. 2 low actionable
11:34	Fix 3 (/fs-fix): ❌ FAILED — post-script rejected commit with Signed-off-by trailer
11:56	Human adds CRITICAL: Do NOT use Signed-off-by to /fs-fix command
12:08	Fix 3 retry: deduplicates env-var precedence, adds cross-ref comments, updates docs. Successful
12:08–12:49	Review 5: APPROVED
13:24	Fix 4 (/fs-fix): ❌ FAILED — Signed-off-by trailer again, despite explicit human prohibition
14:06–15:17	Reviews 6–7: APPROVED. Low findings only (error handling, comment style, type placement)
16:24–17:08	Review 8 (human-triggered /fs-review): APPROVED. All low/informational
17:37	Human approves

Totals: 1 triage + 1 code + 8 review + 5 fix (3 success, 2 failed) = 15 agent runs.

What went well

• Review quality was strong. All three medium-severity findings were genuine bugs: (1) hardcoded port 443 breaking non-standard GitLab instances, (2) duplicated URL-parsing logic that would diverge, (3) inconsistent gating between the two sandbox layers. The human reviewer added no independent code findings — the review agent covered all substantive issues.
• Code agent produced a sound initial implementation. The core architecture (two-layer approach, GitLab detection gating, user-entry preservation) was correct. Review findings were edge-case improvements, not fundamental design issues.
• Fix agent made sound architectural decisions when it succeeded — e.g., extracting shared ResolveForgeHostPort() into internal/forge/gitlab/env.go, replacing fmt.Sprintf YAML templates with yaml.v3 marshaling.

Evidence for existing issues

• fullsend-ai/agents#999 (Fix agent Signed-off-by validation): Two fix runs failed with identical post-script rejections for Signed-off-by trailers. The second failure occurred even after the human added an explicit CRITICAL prohibition in the /fs-fix command. Each failure wasted ~$3 and ~15 min. The fix agent's harness config does include validate-code-output.sh in its validation loop — the check may not be matching the fix agent's commit format, or the max 2 iterations may be exhausted before the trailer is caught.
• fullsend-ai/fullsend#2959 (Finding dedup across re-review iterations): The review agent's security analysis produced ~20 inline "no vulnerability found" confirmations across 8 review cycles ("No SSRF bypass identified", "No privilege escalation", "No fail-open path"). Each required human acknowledgment. These were re-emitted on every cycle because they referenced the current commit SHA, but the analysis conclusions were identical.
• fullsend-ai/fullsend#2655 (Suppress self-negating findings): The security confirmations posted as inline comments were not findings — they were analyses that concluded the code is safe. Suppressing these would have eliminated ~60% of the inline comment volume.
• fullsend-ai/fullsend#3025 (Diminishing value after approval): Reviews 5–8 surfaced only low-severity findings (error handling, comment style, doc-comment consistency, type placement) on code that existed since Review 3. The review agent had already APPROVED by Review 2. These late findings added marginal value relative to their cost (~$32 across 4 review runs).
• fullsend-ai/fullsend#6210 (Autonomy readiness for review agent approvals): The human reviewer approved without adding any independent code findings. All substantive issues were identified by the review agent. This PR is evidence that the review agent could safely auto-approve once its own findings are addressed, at least for well-scoped bot-authored bug fixes with test coverage above the 80% threshold.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 8:09 PM UTC · Completed 8:36 PM UTC

Commit: da1827d · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.87

[fullsend-ai-retro]

Retro: PR #6616 — GitLab forge egress fix

Workflow: 8 review cycles, 5 fix attempts (2 failed), ~$88 across 15+ agent runs for a moderate 14-file GitLab forge egress fix (run timeline). Issue created at 00:41 UTC, PR merged at 20:08 UTC (~19 hours).

The first retro (pre-merge) identified the key improvement areas and referenced existing issues. This post-merge retro confirms those findings and adds specific root-cause detail.

Evidence for existing issues

• #5419 (post-script scan-range rebase safety): The 2 fix failures share a specific root cause. post-fix.sh (line ~1391) uses PRE_AGENT_HEAD..HEAD for the Signed-off-by trailer check, but after a rebase PRE_AGENT_HEAD is orphaned and git log over that range captures all rebased commits — including prior human-authored commits with legitimate Signed-off-by trailers. Meanwhile, validate-code-output.sh (lines 883-887) already uses MERGE_BASE..HEAD for the same check and works correctly. The fix is to align post-fix.sh to use MERGE_BASE..HEAD (already computed at line ~1345) for commit-message inspection.

• #2655 (suppress self-negating findings): 10 of 15 non-actionable review findings were security audit confirmations (SSRF analysis, fail-open analysis, input-validation analysis) posted as inline comments. Each concluded "no vulnerability identified" but required human triage. These accounted for ~60% of inline comment volume.

• #2959 (finding dedup across re-reviews): The review agent posted ~37 inline comments for 28 unique findings across 8 review cycles, re-posting informational findings on unchanged code.

• #6210 (autonomy readiness): The human reviewer found nothing the review agent missed — 100% of human review was dispositional ("Fixed" or "Acknowledged" replies). The agent caught 3 genuine medium-severity bugs (hardcoded port, duplicated URL-parsing, inconsistent guard condition) and achieved 72% precision on non-audit findings.

• fullsend-ai/agents#999 (fix agent Signed-off-by): The code agent's original commits introduced Signed-off-by trailers (violating AGENTS.md), which cascaded through rebases. The fix agent's own commits were clean — the trailers came from rebased prior commits that the scan range incorrectly included.

No new proposals

All improvement areas identified in this retro are already tracked by open issues. The most impactful fix would be #5419 (scan-range alignment), which would have prevented the 2 fix failures (~$8 wasted, ~30 min delay) and eliminated the need for escalating human warnings.