Skip to content

feat(#5838): separate workflow-host allow-list from caller allow-list - #5916

Merged
ifireball merged 7 commits into
mainfrom
agent/5838-workflow-host-allow-list
Aug 5, 2026
Merged

feat(#5838): separate workflow-host allow-list from caller allow-list#5916
ifireball merged 7 commits into
mainfrom
agent/5838-workflow-host-allow-list

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Separates workflow-host trust from caller enrollment in the mint by introducing a dedicated WORKFLOW_HOST_REPOS environment variable. Per-repo callers now have their job_workflow_ref validated against this allow-list (defaulting to fullsend-ai/fullsend), while per-org callers are hard-wired to accept only {org}/.fullsend and upstream workflows. This decouples the two concerns so granting a repo caller access no longer implicitly trusts it as a workflow host.

Changes

  • internal/mintcore/claims.go: Refactored ValidateWorkflowRef to accept isPerRepo and workflowHostRepos parameters. Per-repo mode validates against the host allow-list; per-org mode hard-wires to .fullsend and upstream.
  • internal/mintcore/handler.go: Added workflowHostRepos field to Handler. WORKFLOW_HOST_REPOS is parsed at construction (defaults to fullsend-ai/fullsend). The workflow ref validation now runs after the dual-enrollment guard so the correct mode is used. Status endpoint includes workflow_host_repos in response.
  • internal/mintcore/config.go: Added WorkflowHostRepos to WorkerConfig and NewHandlerFromConfig.
  • internal/cli/mint.go: Added fullsend mint workflow-host add|remove|list CLI commands for managing the allow-list. Updated fullsend mint status to display workflow host repos.
  • internal/dispatch/gcf/provisioner.go: Added AddWorkflowHostRepo and RemoveWorkflowHostRepo methods.
  • internal/dispatch/cf/workersrc/src/index.ts: Added WORKFLOW_HOST_REPOS to Worker env and config.
  • docs/ADRs/0082-workflow-host-allow-list.md: ADR documenting the trust separation.
  • docs/architecture.md: Added cross-reference to ADR 0082.
  • Embed files synced for claims.go, handler.go, config.go.

Testing

  • All internal/mintcore tests pass (including updated tests for the new validation logic)
  • Tests added for per-repo workflow host validation (accept listed host, reject unlisted)
  • Tests added for per-org hard-wired validation (accept .fullsend/upstream, reject others)
  • Tests updated for dual-enrollment interaction with workflow host validation
  • Public mode tests verified (unchanged behavior)
  • go vet passes
  • go build ./cmd/fullsend/... succeeds

Checklist


Closes #5838

Post-script verification

  • Branch is not main/master (agent/5838-workflow-host-allow-list)
  • Secret scan passed (gitleaks — 9df3b1398df9f29db3eada0607b4c9f06535bcb0..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Introduce WORKFLOW_HOST_REPOS env var that controls which repos may
host workflows calling the mint for per-repo callers (default:
fullsend-ai/fullsend). Per-org callers hard-wire to {org}/.fullsend
and upstream. This decouples caller enrollment (PER_REPO_WIF_REPOS /
ALLOWED_ORGS) from workflow-host trust.

Key changes:
- ValidateWorkflowRef now takes isPerRepo, workflowHostRepos params
- Per-repo mode checks workflow host against WORKFLOW_HOST_REPOS
- Per-org mode hard-wires to .fullsend and upstream only
- GET /v1/status includes workflow_host_repos in response
- New CLI: fullsend mint workflow-host add|remove|list
- Provisioner gains AddWorkflowHostRepo/RemoveWorkflowHostRepo
- ADR 0082 documents the trust separation
- WorkerConfig and CF Worker adapter pass WorkflowHostRepos

Note: pre-commit could not run in sandbox (network restriction).
The post-script runs pre-commit authoritatively on the runner.

Closes #5838
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 4, 2026 21:13
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 4, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:14 PM UTC · Completed 9:30 PM UTC
Commit: fc61caf · View workflow run →

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Site preview

Preview: https://300c468d-site.fullsend-ai.workers.dev

Commit: 059d6322241b9aee1bf351549a80ab330015abaf

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.71255% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/mint.go 92.85% 8 Missing and 6 partials ⚠️
internal/dispatch/gcf/provisioner.go 92.15% 3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [backward-incompatible] internal/mintcore/claims.go — Per-repo callers that previously hosted their own workflows (accepted by the old ValidateWorkflowRef which checked the caller's own repo from PER_REPO_WIF_REPOS) will be rejected unless added to WORKFLOW_HOST_REPOS or they happen to be dual-enrolled. Public mode (PER_REPO_WIF_REPOS=*) now enforces WORKFLOW_HOST_REPOS and ALLOWED_WORKFLOW_FILES basename allowlist, whereas previously it skipped basename checking. The default WORKFLOW_HOST_REPOS value (fullsend-ai/fullsend) preserves upstream-workflow behavior. ADR 0082 documents the migration requirement.
    Remediation: Operators with per-repo callers that self-host workflows should add those repos to WORKFLOW_HOST_REPOS using fullsend mint workflow-host add before upgrading.

  • [edge-case] internal/dispatch/gcf/provisioner.goAddWorkflowHostRepo now uses mintcore.SplitCSV for parsing (resolving the prior medium finding). RemoveWorkflowHostRepo uses strings.Split with manual trim+filter producing equivalent results. Both functions now handle malformed CSV values correctly. Stylistic asymmetry remains but has no correctness impact.

  • [missing-cross-reference] docs/ADRs/0059-public-mint-mode-with-wildcard-allowlists.md — ADR 0059 states "Basename gate: that restriction was dropped." ADR 0082 supersedes this exception and notes the supersession in both its Public mode section and Related ADRs table. ADR 0059 itself has no forward reference, but modifying accepted ADRs may conflict with ADR immutability rules.

  • [information-disclosure] internal/mintcore/handler.go/v1/status endpoint now exposes the WORKFLOW_HOST_REPOS list. The endpoint requires full authentication (OIDC + org authorization + workflow ref validation). Values are public GitHub repo slugs, not secrets.

  • [function-signature-change] internal/mintcore/claims.goValidateWorkflowRef signature changed from (ref, repository string, perRepoWIFRepos map[string]bool, allowedWorkflowFiles []string) to (ref, repository string, isPerRepo bool, workflowHostRepos map[string]bool, allowedWorkflowFiles []string). All internal call sites updated. Go internal/ package convention prevents external import.

  • [function-signature-change] internal/mintcore/config.goNewHandlerFromConfig adds workflowHostReposCSV parameter. All internal callers updated.

  • [api-response-schema-change] internal/mintcore/handler.gostatusResponse adds workflow_host_repos JSON field to /v1/status. Backward-compatible (additive, uses omitempty).

  • [new-environment-variable-contract] internal/mintcore/config.go — New WORKFLOW_HOST_REPOS env var introduced. Defaults to fullsend-ai/fullsend when unset, preserving existing behavior.

  • [wasm-config-schema-change] internal/dispatch/cf/workersrc/src/index.ts — Cloudflare Worker Env interface adds optional WORKFLOW_HOST_REPOS? binding. Backward-compatible (optional field, defaults to empty string).


Labels: PR adds new mint authorization feature (workflow-host allow-list separation) implemented in Go with CLI commands, matching type/feature and go labels.

Previous run

Review

Findings

Medium

  • [edge-case] internal/dispatch/gcf/provisioner.go:1694AddWorkflowHostRepo uses strings.Split(existing, ",") without filtering empty entries, while RemoveWorkflowHostRepo correctly skips empty entries via if entry == "" { continue }. If the env var contains malformed values (leading/trailing commas, double commas), AddWorkflowHostRepo preserves them when appending. This asymmetry means malformed values can accumulate but won't cause incorrect authorization decisions (the Go-side SplitCSV handles them correctly).
    Remediation: Use mintcore.SplitCSV for parsing the existing value in AddWorkflowHostRepo, then rejoin and append, keeping the stored value normalized.

Low

  • [backward-incompatible behavior change] internal/mintcore/claims.go:164 — Per-repo callers that previously hosted their own workflows (accepted by the old ValidateWorkflowRef which checked the caller's own repo from PER_REPO_WIF_REPOS) will be rejected unless added to WORKFLOW_HOST_REPOS or they happen to be dual-enrolled. The dual-enrollment upgrade in handler.go mitigates this for callers whose org is in ALLOWED_ORGS, but pure per-repo callers (org not in ALLOWED_ORGS) hosting their own workflows will break. The ADR documents this as a migration requirement (ADR 0082 Consequences bullet 3), and the default WORKFLOW_HOST_REPOS value preserves upstream-workflow behavior.
    Remediation: Consider adding a startup-time log warning when PER_REPO_WIF_REPOS contains entries that are not in WORKFLOW_HOST_REPOS, alerting operators.

  • [backward-incompatible behavior change] internal/mintcore/claims.go:150 — Public mint mode (PER_REPO_WIF_REPOS=*) now enforces WORKFLOW_HOST_REPOS and ALLOWED_WORKFLOW_FILES basename allowlist, whereas previously it only enforced upstream repo and skipped basename checking. Public mode now uses the same per-repo validation path (ADR 0082 §2). The default WORKFLOW_HOST_REPOS includes fullsend-ai/fullsend, preserving the upstream-only constraint.
    Remediation: Document in release notes that public mint mode now enforces ALLOWED_WORKFLOW_FILES and WORKFLOW_HOST_REPOS.

  • [architectural-coherence] docs/ADRs/0082-workflow-host-allow-list.md — ADR 0082 re-introduces basename gating for public mode via the per-repo validation path (ALLOWED_WORKFLOW_FILES applies in public mode), which contradicts ADR 0059's Consequences statement: "Basename gate: that restriction was dropped." ADR 0082 does not cross-reference ADR 0059 as amended. Per ADR immutability rules, ADR 0059 should not be modified; instead ADR 0082 should note that it supersedes ADR 0059's basename gate exception.
    Remediation: Add a note in ADR 0082 that it supersedes ADR 0059's basename gate exception for public mode, and optionally add ADR 0059 to the Related ADRs table.

  • [test-adequacy] internal/mintcore/config_test.go — No test verifies non-empty workflowHostReposCSV parsing or the default fallback in NewHandlerFromConfig. All test calls pass empty string for the new parameter.
    Remediation: Add a test similar to TestNewHandlerFromConfig_PerRepoWIFReposCSV that passes non-empty workflowHostReposCSV, inspects h.workflowHostRepos.

  • [function-signature-change] internal/mintcore/claims.go:165ValidateWorkflowRef function signature changed from (ref, repository string, perRepoWIFRepos map[string]bool, allowedWorkflowFiles []string) to (ref, repository string, isPerRepo bool, workflowHostRepos map[string]bool, allowedWorkflowFiles []string). All internal call sites are updated. Go internal/ package convention prevents external import.

  • [function-signature-change] internal/mintcore/config.goNewHandlerFromConfig function signature changed by adding a new workflowHostReposCSV parameter. All internal callers updated. Go internal/ package convention prevents external import.

  • [api-response-schema-change] internal/mintcore/handler.gostatusResponse adds a new workflow_host_repos JSON field to /v1/status. Backward-compatible (additive, uses omitempty).

  • [new-environment-variable-contract] internal/mintcore/config.go — New WORKFLOW_HOST_REPOS env var introduced. Defaults to fullsend-ai/fullsend when unset, preserving existing behavior.

  • [wasm-config-schema-change] internal/dispatch/cf/workersrc/src/index.ts — Cloudflare Worker Env interface adds new WORKFLOW_HOST_REPOS? binding and buildWasmConfig includes it. Backward-compatible (optional field, defaults to empty string).

Previous run (2)

Review

Findings

Low

  • [backward-incompatible behavior change] internal/mintcore/claims.go:164 — Per-repo callers that previously hosted their own workflows (accepted by the old ValidateWorkflowRef which checked the caller's own repo from PER_REPO_WIF_REPOS) will be rejected unless added to WORKFLOW_HOST_REPOS or they happen to be dual-enrolled. The dual-enrollment upgrade in handler.go mitigates this for callers whose org is in ALLOWED_ORGS, but pure per-repo callers (org not in ALLOWED_ORGS) hosting their own workflows will break. The ADR documents this as a migration requirement (ADR 0082 Consequences bullet 3), and the default WORKFLOW_HOST_REPOS value preserves upstream-workflow behavior.
    Remediation: Consider adding a startup-time log warning when PER_REPO_WIF_REPOS contains entries that are not in WORKFLOW_HOST_REPOS, alerting operators.

  • [architectural-coherence] docs/ADRs/0082-workflow-host-allow-list.md — ADR 0082 re-introduces basename gating for public mode via the per-repo validation path (ALLOWED_WORKFLOW_FILES applies in public mode), which contradicts ADR 0059's Consequences statement: "Basename gate: that restriction was dropped." ADR 0082 does not cross-reference ADR 0059 as amended. Per ADR immutability rules, ADR 0059 should not be modified; instead ADR 0082 should note that it supersedes ADR 0059's basename gate exception.
    Remediation: Add a note in ADR 0082 that it supersedes ADR 0059's basename gate exception for public mode, and optionally add ADR 0059 to the Related ADRs table.

  • [test-adequacy] internal/mintcore/config_test.go — No test verifies non-empty workflowHostReposCSV parsing or the default fallback in NewHandlerFromConfig. All test calls pass empty string for the new parameter.
    Remediation: Add a test similar to TestNewHandlerFromConfig_PerRepoWIFReposCSV that passes non-empty workflowHostReposCSV, inspects h.workflowHostRepos.

  • [api-response-schema-change] internal/mintcore/handler.gostatusResponse adds a new workflow_host_repos JSON field to /v1/status. Backward-compatible (additive, uses omitempty).

  • [new-environment-variable-contract] internal/mintcore/config.go — New WORKFLOW_HOST_REPOS env var introduced. Defaults to fullsend-ai/fullsend when unset, preserving existing behavior.

  • [design-observation] internal/mintcore/claims.go:177 — In per-repo mode, upstream fullsend-ai/fullsend is unconditionally trusted via hardcoded check before consulting workflowHostRepos. If an operator sets WORKFLOW_HOST_REPOS to only their repos expecting to exclude upstream, the hardcoded check still accepts upstream. ADR 0082 documents this as intended behavior ("always accepted regardless of workflowHostRepos contents").

Previous run (3)

Review

Findings

Low

  • [backward-incompatible behavior change] internal/mintcore/claims.go:165 — Per-repo callers that previously hosted their own workflows (accepted by the old ValidateWorkflowRef which checked the caller's own repo from PER_REPO_WIF_REPOS) will be rejected unless added to WORKFLOW_HOST_REPOS or they happen to be dual-enrolled. The dual-enrollment upgrade in handler.go mitigates this for callers whose org is in ALLOWED_ORGS, but pure per-repo callers (org not in ALLOWED_ORGS) hosting their own workflows will break. The ADR documents this as a migration requirement (ADR 0082 Consequences bullet 3), and the default WORKFLOW_HOST_REPOS value preserves upstream-workflow behavior.
    Remediation: Consider adding a startup-time log warning when PER_REPO_WIF_REPOS contains entries that are not in WORKFLOW_HOST_REPOS, alerting operators.

  • [architectural-coherence] docs/ADRs/0082-workflow-host-allow-list.md — ADR 0082's revised public-mode handling (section 2, revised 2026-08-05) changes public mode from upstream-only with no basename gate to per-repo validation path with WORKFLOW_HOST_REPOS and basename gate, superseding ADR 0059's statements about public mode. ADR 0059 explicitly dropped the basename gate. ADR 0082 does not cross-reference ADR 0059 as amended.
    Remediation: Add amendment note to ADR 0059 referencing ADR 0082's public-mode change.

  • [code-duplication] internal/cli/mint.go:1587 — The comma-separated WORKFLOW_HOST_REPOS parsing logic is duplicated between runMintStatus and newMintWorkflowHostListCmd. Both inline strings.Split/TrimSpace loops rather than calling mintcore.SplitCSV, which already provides this exact behavior.
    Remediation: Replace both inline parsing blocks with mintcore.SplitCSV(raw).

  • [naming-consistency] docs/architecture.md:122 — The new bullet uses a tab-indented sub-item whereas surrounding items at this level are top-level bullets in a flat list. This makes it a child of the preceding "Mint repos scope" bullet rather than a sibling.
    Remediation: Remove the leading tab so the bullet aligns with the other items in the list.

  • [api-response-schema-change] internal/mintcore/handler.go:287statusResponse adds a new workflow_host_repos JSON field to /v1/status. Backward-compatible (additive, uses omitempty).

  • [new-environment-variable-contract] internal/mintcore/config.go:883 — New WORKFLOW_HOST_REPOS env var introduced. Defaults to fullsend-ai/fullsend when unset, preserving existing behavior.


Labels: PR separates workflow-host trust from caller enrollment in the mint authorization system, which is a security-relevant trust boundary change.

Previous run (4)

Review

Findings

Medium

  • [backward-incompatible behavior change] internal/mintcore/claims.go:190 — The old ValidateWorkflowRef accepted per-repo callers hosting their own workflows when the caller's repo was listed in perRepoWIFRepos. The new code validates only against workflowHostRepos (defaulting to fullsend-ai/fullsend). Existing per-repo callers that host their own workflows will be rejected unless added to WORKFLOW_HOST_REPOS. The ADR documents this as a migration requirement, but there is no runtime migration guard (e.g., a log warning when a request would have succeeded under the old policy but fails under the new one) to help operators discover affected callers.
    Remediation: Consider adding a temporary compatibility log — when a per-repo caller's workflow ref would have matched under the old perRepoWIFRepos-based check but fails under the new workflowHostRepos check, log a warning (but still deny).

Low

  • [Input Validation] internal/dispatch/gcf/provisioner.go:5096AddWorkflowHostRepo validates owner/repo format and rejects commas, but does not validate individual owner/repo components against existing patterns (GitHubOrgPattern, ValidateRepoSlug). Malformed values would be no-op entries (never matching a real workflow ref) rather than a privilege escalation.
    Remediation: Add ValidateRepoSlug and GitHubOrgPattern validation to the owner and repo components for consistency with enrollment commands.

  • [naming-consistency] docs/architecture.md:115 — The new bullet uses a tab-indented sub-item whereas surrounding items at this level are top-level bullets in a flat list. This makes it a child of the preceding "Mint repos scope" bullet rather than a sibling.
    Remediation: Remove the leading tab so the bullet aligns with the other items in the list.

  • [code-duplication] internal/cli/mint.go:1577 — The comma-separated WORKFLOW_HOST_REPOS parsing logic is duplicated between runMintStatus and newMintWorkflowHostListCmd. The same logic already exists as mintcore.SplitCSV.
    Remediation: Reuse mintcore.SplitCSV to consolidate the two copies.

  • [Information Disclosure] internal/mintcore/handler.go:668 — The /v1/status endpoint now returns the workflowHostRepos list to authenticated callers, expanding the response's information surface. The endpoint requires full authentication and ADR 0082 explicitly specifies this field, so the exposure is intentional.

  • [api-response-schema-change] internal/mintcore/handler.go:299statusResponse adds a new workflow_host_repos JSON field to /v1/status. Backward-compatible (additive, uses omitempty).

  • [new-environment-variable-contract] internal/mintcore/config.go:945 — New WORKFLOW_HOST_REPOS env var introduced. Defaults to fullsend-ai/fullsend when unset, preserving existing behavior.

Previous run (5)

Review

Findings

Medium

  • [backward-incompatible behavior change] internal/mintcore/claims.go:186 — The old ValidateWorkflowRef accepted per-repo callers hosting their own workflows when the caller's repo was listed in perRepoWIFRepos. The new code removes this fallback: per-repo callers' workflow refs are validated only against workflowHostRepos (which defaults to fullsend-ai/fullsend). Existing per-repo callers that host their own workflows will start getting rejected unless an operator adds their repo to WORKFLOW_HOST_REPOS. The ADR documents this as a migration requirement, but there is no runtime migration guard (e.g., a log warning when a request would have succeeded under the old policy but fails under the new one) to help operators discover affected callers.
    Remediation: Consider adding a temporary compatibility log — when a per-repo caller's workflow ref would have matched under the old perRepoWIFRepos-based check but fails under the new workflowHostRepos check, log a warning (but still deny). This gives operators visibility into which callers need WORKFLOW_HOST_REPOS enrollment.

  • [missing-documentation] docs/guides/infrastructure/infrastructure-reference.md — The Infrastructure Reference guide documents mint security controls and lists env vars like ALLOWED_ORGS, ALLOWED_WORKFLOW_FILES, and PER_REPO_WIF_REPOS, but does not document WORKFLOW_HOST_REPOS. The workflow ref validation section describes the old logic (accepts registered per-repo workflows via PER_REPO_WIF_REPOS), which is now factually incorrect.
    Remediation: Add WORKFLOW_HOST_REPOS to the mint security controls section and update workflow ref validation documentation to describe the new per-repo vs per-org validation logic.

Low

  • [unnecessary side effect] internal/dispatch/gcf/provisioner.go:1724RemoveWorkflowHostRepo always calls UpdateServiceEnvVars, even when the target repo was not found in WORKFLOW_HOST_REPOS. This creates a new Cloud Run revision with identical env vars. AddWorkflowHostRepo correctly returns early (nil) when the repo is already present, but RemoveWorkflowHostRepo lacks the symmetric early-return when the repo is absent.
    Remediation: After filtering, compare the filtered result to the existing value. If unchanged, return nil without calling UpdateServiceEnvVars.

  • [missing-documentation] docs/guides/infrastructure/mint-administration.md — The mint administration guide does not document the new fullsend mint workflow-host subcommand (add/remove/list).

  • [missing-documentation] docs/cli/mint.md — The CLI reference for fullsend mint does not include the new workflow-host subcommand.

  • [missing-documentation] docs/guides/infrastructure/standalone-mint.md — The standalone mint guide documents PER_REPO_WIF_REPOS but not WORKFLOW_HOST_REPOS.

  • [api-response-schema-change] internal/mintcore/handler.gostatusResponse adds a new workflow_host_repos JSON field to the /v1/status endpoint response. This is backward-compatible (additive, uses omitempty).


Labels: PR modifies mint authorization logic (mintcore, CLI, provisioner) and adds a security-related trust separation feature

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/mint Token mint and cross-boundary credentials labels Aug 4, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix the review comments, but ignore the behaviour change complaints, that change is intentional

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 5:41 AM UTC · Completed 5:50 AM UTC
Commit: fc61caf · View workflow run →

- Add early return in RemoveWorkflowHostRepo when target repo is not
  found, avoiding unnecessary Cloud Run revision creation (matches
  AddWorkflowHostRepo pattern)
- Document WORKFLOW_HOST_REPOS in infrastructure-reference.md and update
  workflow ref validation to describe per-org vs per-repo logic
- Document workflow-host add|remove|list subcommand in
  mint-administration.md and docs/cli/mint.md
- Document WORKFLOW_HOST_REPOS in standalone-mint.md

Addresses review feedback on #5916
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 5 of 7 review findings. Fixed RemoveWorkflowHostRepo early-return bug and added missing documentation for WORKFLOW_HOST_REPOS and workflow-host subcommand across 4 docs files. Disagreed with 2 findings: backward-incompatible behavior change (intentional per human instruction) and api-response-schema-change (informational note, no action needed).

Fixed (5):

  1. missing-documentation in infrastructure-reference.md (docs/guides/infrastructure/infrastructure-reference.md): Added WORKFLOW_HOST_REPOS to Mint Security Controls section. Updated workflow ref validation to describe separate per-org (hard-wired to .fullsend and upstream) vs per-repo (validated against WORKFLOW_HOST_REPOS) logic. Added workflow-host to managed-by line.
  2. unnecessary side effect in RemoveWorkflowHostRepo (internal/dispatch/gcf/provisioner.go): Added early return when the target repo is not found in WORKFLOW_HOST_REPOS, avoiding unnecessary Cloud Run revision creation. Matches the symmetric early-return pattern in AddWorkflowHostRepo.
  3. missing-documentation in mint-administration.md (docs/guides/infrastructure/mint-administration.md): Added workflow-host add|remove|list to command table and a new 'Managing workflow hosts' section with usage examples and flags.
  4. missing-documentation in docs/cli/mint.md (docs/cli/mint.md): Added workflow-host add|remove|list to the commands table and a new 'mint workflow-host' section with subcommand usage and flags.
  5. missing-documentation in standalone-mint.md (docs/guides/infrastructure/standalone-mint.md): Added WORKFLOW_HOST_REPOS to the optional variables table with description and example.

Disagreed (2):

  1. backward-incompatible behavior change in ValidateWorkflowRef: Human instruction: this behavior change is intentional. The ADR documents the migration requirement. No compatibility log is needed.
  2. api-response-schema-change in statusResponse: The review noted this is backward-compatible (additive, uses omitempty). No action requested or needed.

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

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 5:50 AM UTC · Ended 6:06 AM UTC
Commit: faed602 · View workflow run →

@ifireball

Copy link
Copy Markdown
Member

/fs-fix lets avoid special-casing public mode - it should just behave like per-repo mode and support the workflow host repos list. The only difference between that and tight per-repo mode should be the checking of requesting repos against the respective allow list or lack thereof. Add ADR revision refereces where needed to specify this.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 5:59 AM UTC · Completed 6:11 AM UTC
Commit: faed602 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:50 AM UTC · Completed 6:06 AM UTC
Commit: faed602 · View workflow run →

Public mode (PER_REPO_WIF_REPOS=*) now uses the same per-repo
validation path with WORKFLOW_HOST_REPOS and the basename allowlist.
The only difference between public and tight per-repo mode is caller
enrollment. Removes the perRepoWIFRepos parameter from
ValidateWorkflowRef as it was only used for the now-removed
public-mode early return. ADR 0082 updated with revision note.

Addresses review feedback on #5916
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Removed public-mode special-case from ValidateWorkflowRef per human instruction. Public mode now uses the same per-repo validation path with WORKFLOW_HOST_REPOS and ALLOWED_WORKFLOW_FILES basename gate. Updated ADR 0082, architecture.md, infrastructure-reference.md, standalone-mint.md with revision notes. All tests pass.

Fixed (6):

  1. remove public-mode special-case from ValidateWorkflowRef (internal/mintcore/claims.go): Removed the IsPublicMintRepos early return that special-cased public mode. Public mode now uses the same per-repo validation path with workflowHostRepos and basename allowlist. Removed the unused perRepoWIFRepos parameter from the function signature.
  2. update ValidateWorkflowRef call sites in handler (internal/mintcore/handler.go): Updated both ValidateWorkflowRef call sites (v1/status and v1/token paths) to remove the perRepoWIFRepos argument.
  3. update public mode unit tests (internal/mintcore/claims_test.go): Rewrote TestValidateWorkflowRef_PublicMode to verify that public mode uses workflow host repos and basename allowlist. Updated all ValidateWorkflowRef call sites to match new signature.
  4. update public mode handler integration tests (internal/mintcore/handler_test.go): Updated TestHandler_PublicMintMode to use an allowed workflow basename (dispatch.yml) since public mode now enforces the basename allowlist.
  5. sync embed files (internal/dispatch/gcf/mintsrc/mintcore/claims.go.embed): Synced claims.go.embed and handler.go.embed with the updated claims.go and handler.go source files.
  6. add ADR revision references (docs/ADRs/0082-workflow-host-allow-list.md): Updated ADR 0082 Public mode section to describe the new behavior and added a revision note dated 2026-08-05. Updated architecture.md, infrastructure-reference.md, and standalone-mint.md with ADR 0082 cross-references and corrected public mode documentation.

Tests: passed

Decision points
  • Removed the perRepoWIFRepos parameter from ValidateWorkflowRef entirely rather than leaving it unused (alternatives: Leave unused parameter for backward compatibility, Remove unused parameter for cleanliness; rationale: The parameter was only used for the public mode special-case check (IsPublicMintRepos). With that removed, keeping the parameter would be dead code and confusing to future readers.)

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

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:12 AM UTC · Completed 6:27 AM UTC
Commit: 74aab9e · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge security Security threat model and related concerns and removed requires-manual-review Review requires human judgment labels Aug 5, 2026
Comment thread docs/ADRs/0082-workflow-host-allow-list.md Outdated
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels Aug 5, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix

  • [edge-case] internal/dispatch/gcf/provisioner.go:1694AddWorkflowHostRepo uses strings.Split(existing, ",") without filtering empty entries, while RemoveWorkflowHostRepo correctly skips empty entries via if entry == "" { continue }. If the env var contains malformed values (leading/trailing commas, double commas), AddWorkflowHostRepo preserves them when appending. This asymmetry means malformed values can accumulate but won't cause incorrect authorization decisions (the Go-side SplitCSV handles them correctly).
    Remediation: Use mintcore.SplitCSV for parsing the existing value in AddWorkflowHostRepo, then rejoin and append, keeping the stored value normalized.

  • [architectural-coherence] docs/ADRs/0082-workflow-host-allow-list.md — ADR 0082 re-introduces basename gating for public mode via the per-repo validation path (ALLOWED_WORKFLOW_FILES applies in public mode), which contradicts ADR 0059's Consequences statement: "Basename gate: that restriction was dropped." ADR 0082 does not cross-reference ADR 0059 as amended. Per ADR immutability rules, ADR 0059 should not be modified; instead ADR 0082 should note that it supersedes ADR 0059's basename gate exception.
    Remediation: Add a note in ADR 0082 that it supersedes ADR 0059's basename gate exception for public mode, and optionally add ADR 0059 to the Related ADRs table.

  • [test-adequacy] internal/mintcore/config_test.go — No test verifies non-empty workflowHostReposCSV parsing or the default fallback in NewHandlerFromConfig. All test calls pass empty string for the new parameter.
    Remediation: Add a test similar to TestNewHandlerFromConfig_PerRepoWIFReposCSV that passes non-empty workflowHostReposCSV, inspects h.workflowHostRepos.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 10:48 AM UTC · Completed 10:56 AM UTC
Commit: e94444f · View workflow run →

- Use mintcore.SplitCSV in AddWorkflowHostRepo to normalize existing
  WORKFLOW_HOST_REPOS value, matching RemoveWorkflowHostRepo's handling
  of empty entries from malformed CSV values
- Add note in ADR 0082 superseding ADR 0059's basename gate exception
  for public mode, with Related ADRs table cross-referencing both
- Add tests for non-empty workflowHostReposCSV parsing and default
  fallback in NewHandlerFromConfig

Addresses review feedback on #5916
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 5 (human-triggered)

Addressed all 3 review findings: normalized CSV parsing in AddWorkflowHostRepo via mintcore.SplitCSV, added ADR 0059 cross-reference in ADR 0082, and added test coverage for workflowHostReposCSV parsing and default fallback.

Fixed (3):

  1. AddWorkflowHostRepo uses strings.Split without filtering empty entries (internal/dispatch/gcf/provisioner.go): Replaced strings.Split with mintcore.SplitCSV to normalize the existing WORKFLOW_HOST_REPOS value when checking for duplicates and appending. This matches RemoveWorkflowHostRepo's handling and prevents malformed values (leading/trailing commas, double commas) from accumulating.
  2. ADR 0082 re-introduces basename gating without cross-referencing ADR 0059 (docs/ADRs/0082-workflow-host-allow-list.md): Added a note in ADR 0082's Public mode section explicitly stating it supersedes ADR 0059's basename gate exception. Added a Related ADRs table cross-referencing ADR 0078 and ADR 0059.
  3. No test for non-empty workflowHostReposCSV in NewHandlerFromConfig (internal/mintcore/config_test.go): Added TestNewHandlerFromConfig_WorkflowHostReposCSV (verifies parsing and lowercasing of non-empty CSV) and TestNewHandlerFromConfig_WorkflowHostReposCSV_DefaultFallback (verifies default to fullsend-ai/fullsend when empty), mirroring the existing TestNewHandlerFromConfig_PerRepoWIFReposCSV pattern.

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

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 10:57 AM UTC · Ended 11:12 AM UTC
Commit: ca1c1ab · View workflow run →

@ifireball

Copy link
Copy Markdown
Member

/fs-fix when a caller is both an enrolled repo and its org is an enrolled org (matching both per-repo and per-org modes) - both job_workflow_ref options should apply - the workflow could either come from an approved host repo or from the per-org mode worflow sources.

Need to fix the ADR, the docs and the code to match this. Don't forget to sync the embed files

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 11:01 AM UTC · Completed 11:11 AM UTC
Commit: ca1c1ab · View workflow run →

When a caller is both in PER_REPO_WIF_REPOS and its org is in
ALLOWED_ORGS, workflow ref validation now accepts sources from
either per-repo mode (WORKFLOW_HOST_REPOS) or per-org mode
({org}/.fullsend, upstream). Previously dual enrollment forced
per-org validation only, rejecting workflows from approved host
repos.

Updates ADR 0082, infrastructure docs, and syncs embed files.

Addresses human instruction on #5916
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 6 (human-triggered)

Implemented dual-enrollment workflow ref validation: callers matching both PER_REPO_WIF_REPOS and ALLOWED_ORGS now accept workflow refs from either per-repo sources (WORKFLOW_HOST_REPOS) or per-org sources ({org}/.fullsend, upstream). Updated handler.go (both /v1/status and /v1/token paths), claims.go doc comment, ADR 0082, infrastructure-reference and mint-administration docs, embed files, and added comprehensive tests.

Fixed (7):

  1. dual-enrolled callers should accept workflow refs from both per-repo and per-org modes (internal/mintcore/handler.go): Updated both /v1/status and /v1/token paths to try per-org workflow ref validation first, then fall back to per-repo validation for dual-enrolled callers. Previously dual enrollment forced per-org mode only, rejecting workflows from WORKFLOW_HOST_REPOS.
  2. ValidateWorkflowRef doc comment should describe dual-enrollment behavior (internal/mintcore/claims.go): Added doc comment explaining that the handler calls ValidateWorkflowRef twice for dual-enrolled callers — once per-org, then falling back to per-repo.
  3. ADR 0082 should document dual-enrollment workflow ref union behavior (docs/ADRs/0082-workflow-host-allow-list.md): Added 'Dual enrollment' section documenting that both per-repo and per-org workflow sources are accepted when a caller matches both PER_REPO_WIF_REPOS and ALLOWED_ORGS.
  4. infrastructure-reference docs should mention dual-enrollment workflow validation (docs/guides/infrastructure/infrastructure-reference.md): Added bullet point for dual-enrolled callers' workflow ref validation under Mint Security Controls.
  5. mint-administration docs should mention dual-enrollment in workflow host section (docs/guides/infrastructure/mint-administration.md): Updated the Managing workflow hosts section to note that dual-enrolled callers accept workflows from either per-repo or per-org sources.
  6. test coverage for dual-enrollment workflow ref validation (internal/mintcore/handler_test.go): Added TestHandler_DualEnrollment_WorkflowRefAcceptsBothModes with 4 subtests: per-org source (.fullsend repo), per-repo source (workflow host repo), upstream (always accepted), and unlisted repo (rejected).
  7. embed files must be synced (internal/dispatch/gcf/mintsrc/mintcore/handler.go.embed): Synced handler.go.embed and claims.go.embed from their source files.

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

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:12 AM UTC · Completed 11:33 AM UTC
Commit: 059d632 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge type/feature New capability request go Pull requests that update go code and removed requires-manual-review Review requires human judgment labels Aug 5, 2026
@ifireball
ifireball added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit 6ceb2c0 Aug 5, 2026
31 checks passed
@ifireball
ifireball deleted the agent/5838-workflow-host-allow-list branch August 5, 2026 13:10
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 1:12 PM UTC · Completed 1:29 PM UTC
Commit: 059d632 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5916 — Separate workflow-host allow-list from caller allow-list

Timeline

  1. Triage (Aug 2): Issue #5838 triaged successfully.
  2. Code attempt 1 (Aug 4 20:25 UTC, run 30947812165): Failed — code agent wrote correct logic but did not sync mint embed files. Pre-commit hook lint-mint-embed-sync caught the desync and blocked the commit.
  3. Code attempt 2 (Aug 4 20:53 UTC, run 30949888746): Succeeded after human added explicit instruction: "don't forget to sync the mint embed files this time." Created PR feat(#5838): separate workflow-host allow-list from caller allow-list #5916.
  4. Initial review (Aug 4 21:30 UTC): Found 2 medium + 5 low findings. 5 were actionable, 1 intentionally dismissed (backward-incompatible behavior), 1 informational.
  5. Fix iterations 1–6 (Aug 5 05:41–11:08 UTC): 6 fix iterations over ~5.5 hours. 2 were human-directed design changes (remove public-mode special-case; add dual-enrollment logic), 1 was coverage-focused, 1 cleaned up fix agent mistakes, 2 addressed review findings.
  6. Merged (Aug 5 13:10 UTC) by ifireball.

What went well

  • Review agent caught real bugs: The RemoveWorkflowHostRepo unnecessary side effect and AddWorkflowHostRepo CSV parsing asymmetry were genuine correctness issues caught by the review agent.
  • Fix agent followed instructions well: It correctly ignored dismissed findings when told to, implemented design changes when directed, and achieved the 80% coverage target.
  • Pre-commit hook safety net worked: The embed sync desync was caught before the PR was created, preventing a broken PR from being opened.

Evidence for existing open issues

  • #5890 (embed sync guidance): The code agent failed its first run despite go-code.md already documenting the embed sync requirement. This confirms the guidance needs to be more prominent or the sync should be automated (#2965).
  • #4682 / agents#106 / #5265 (dismissed findings re-raised): The review agent raised the backward-incompatible behavior change finding in all 5 review runs, despite the human explicitly saying "ignore the behaviour change complaints" before the first fix. This is one of the clearest examples of the dismissed-finding re-raise problem.
  • agents#464 (fix agent self-review): Fix iteration 2 introduced duplicated CSV parsing (inline strings.Split instead of existing mintcore.SplitCSV), requiring iteration 3 to clean up. This is evidence that the fix agent's self-review doesn't catch pattern-inconsistent idioms.
  • #2287 / #2665 (informational noise): Findings like api-response-schema-change, new-environment-variable-contract, function-signature-change, and wasm-config-schema-change were repeated in every review run. These are change observations rather than actionable issues.

Proposals filed

dhshah13 pushed a commit to dhshah13/fullsend that referenced this pull request Aug 6, 2026
…t revision annotations

Add explicit guidance to the "New ADRs in pull requests" section
of docs/contributing/adrs.md: when editing an ADR that has not
yet merged to main, change the content directly without adding
"Revised" annotations, revision dates, or revision history
sections. This rule was implicit in "Approval happens at merge"
but agents did not infer it, causing a fix agent to add an
unnecessary revision date to a brand-new ADR on PR fullsend-ai#5916.

Note: pre-commit and make lint could not run in the sandbox
(network restriction prevented fetching origin). Manual
verification of lint is required.

Closes fullsend-ai#5932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/mint Token mint and cross-boundary credentials go Pull requests that update go code ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch security Security threat model and related concerns type/feature New capability request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mint: separate caller allow-list from workflow-host allow-list (default fullsend-ai/fullsend)

2 participants