feat: add mintcore shared library and dev mint for local evaluation by ggallen · Pull Request #1751 · fullsend-ai/fullsend

ggallen · 2026-06-01T15:25:39Z

Summary

Introduces a shared mint core library (internal/mintcore/) and a local development mint (internal/devmint/), enabling fullsend to run without GCP infrastructure for token minting. This is a refactoring of the mint architecture to support multiple implementations behind shared interfaces.

mintcore shared library (`internal/mintcore/`)

New Go module extracted from both the GCP Cloud Function mint and the dev mint:

JWKS-based OIDC verifier — direct JWT signature verification using keys from token.actions.githubusercontent.com, replacing GCP STS/WIF token exchange
GitHub API functions — GenerateAppJWT, FindInstallation, CreateInstallationToken
Claim types and validation — Claims, Audience (string-or-array), ValidateOrgAllowed, ValidateWorkflowRef
Shared patterns — GitHubOrgPattern, RepoNamePattern, RolePattern

GCP mint refactoring (`internal/mint/`)

Uses mintcore.OIDCVerifier for OIDC verification instead of GCP STS/WIF
Uses mintcore.* for all shared types and functions
Adds /v1/status diagnostic endpoint (returns configured orgs and roles)
Removes stsTokenValidator, TokenValidator interface, local GitHub API functions, and WIF-related env vars

Dev mint (`internal/devmint/`)

Local HTTP server for development and evaluation:

Disk-based PEM storage with fsnotify hot reload — write PEM files to disk and the mint picks them up automatically
Optional OIDC verification (--insecure-no-auth flag for local development)
cloudflared tunnel support for exposing the local mint to GitHub Actions runners
Removed POST /v1/pem endpoint — the mint is now read-only, PEMs are managed via filesystem

CLI changes

fullsend mint dev command with --insecure-no-auth and --oidc-audience flags
--mint-data-dir flag for disk-based PEM storage during install
storePEMToDisk() replaces postPEMToMint() — PEMs written directly to disk instead of HTTP POST

Cloud Function deployment

Cloud Function provisioner bundles mintcore/ source alongside the mint function
Embedded .embed files for mintcore in mintsrc/mintcore/
go.mod replace directive rewritten from ../mintcore to ./mintcore during zip creation

Changes from review feedback

These changes address review feedback:

GCP-independent OIDC verification — replaced GCP STS/WIF with direct JWKS verification, eliminating the dependency on GCP for token validation
Dropped POST /v1/pem — the mint is now read-only; PEMs are written to disk and hot-reloaded via fsnotify
Added /v1/status endpoint — diagnostic endpoint on the GCP mint returning configured orgs and roles
Shared code via mintcore/ — extracted common code into a shared Go module used by both mint implementations

Test plan

go test ./internal/mintcore/... — 28 tests (JWKS verifier, claims, GitHub API, patterns)
go test ./internal/mint/... — 26 tests (GCP mint with JWKS verifier)
go test ./internal/devmint/... — 22 tests (dev mint, hot reload, no /v1/pem)
go test ./internal/dispatch/gcf/... — provisioner tests with mintcore bundling
go test ./internal/cli/... — CLI tests with disk-based PEM storage
End-to-end: fullsend mint dev → tunnel → fullsend admin install → triage agent runs successfully

🤖 Generated with Claude Code

github-actions · 2026-06-01T15:27:29Z

Site preview

Preview: https://1ee6ea7b-site.fullsend-ai.workers.dev

Commit: 04134860a560bbbb24631a0f013401b1a21959ab

fullsend-ai-review · 2026-06-01T15:31:20Z

Review

Prior findings resolved

✅ [data-exposure] internal/mint/main.go — The GCP mint's /v1/status endpoint now requires Bearer token OIDC verification before returning data. Prior medium-severity finding resolved.

✅ [silent-error] internal/cli/admin.go (storePEMToDisk) — Invalid JSON in config.json now returns an error instead of being silently discarded. Prior medium-severity finding resolved.

Findings

Medium

[race-condition] internal/cli/admin.go:storePEMToDisk — The function performs a non-atomic read-modify-write on config.json (read → unmarshal → modify → write temp → rename). If two concurrent callers race for different roles, one rename could overwrite the other's changes, losing a role entry. All current callers use sequential loops so this is a latent bug, not an active one, but the function has no internal synchronization and could be called concurrently in the future.
Remediation: Add file-level locking (e.g., flock via syscall.Flock) or document that storePEMToDisk must not be called concurrently for the same dataDir.
[data-exposure] internal/devmint/devmint.go:handleStatus — The dev mint's /v1/status endpoint is unauthenticated — it returns the org name, all role names, and app IDs to any caller. When OIDC verification is enabled (no --insecure-no-auth) and the mint is exposed via --tunnel, /v1/status leaks configuration data through the public tunnel URL while /v1/token correctly requires OIDC. The production GCP mint now correctly gates /v1/status behind Bearer token auth, but the dev mint does not mirror this.
Remediation: When OIDC verification is enabled, require a valid OIDC token on /v1/status, matching the production mint's behavior. When --insecure-no-auth is set, the endpoint can remain open since the tunnel guard already prevents that combination.

Low

[logic-error] internal/devmint/devmint.go:loadFromDisk — loadFromDisk writes to s.org, s.pems, and s.appIDs without holding s.mu. Currently safe because it only runs during Start() before the HTTP server starts, and the code explicitly says "restart to pick up changes." However, if hot-reload were added in the future, this would become a data race.
[error-handling] internal/devmint/devmint.go:handleToken — The CLI command newMintRunCmd never sets PerRepoWIFRepos in Options, so the dev mint always uses an empty map. Per-repo workflow refs will always be rejected by ValidateWorkflowRef when OIDC is enabled. This is a minor edge case since --insecure-no-auth is the expected dev flow and standard workflows work fine via the wildcard allowedWorkflows.
[data-exposure] internal/mint/main.go:ServeHTTP — The /v1/status OIDC verification error response includes the error message verbatim (fmt.Sprintf("OIDC verification failed: %v", err)), which may leak internal details (key IDs, issuer URLs, or STS error messages) to callers. Return a generic error message and log details server-side only.
[test-inadequate] internal/mint/main_test.go — Many handler-level OIDC validation tests were removed (prevalidate tests for bad issuer, expired token, bad audience, missing job_workflow_ref, per-repo workflow refs, workflow allowlists). The equivalent logic moved to internal/mintcore/ and is tested there comprehensively, but handler-level integration coverage of the full HTTP request path is thinner.
[error-handling] internal/dispatch/gcf/provisioner.go:addDirToZip — Silently returns nil if the mintcore source directory does not exist. If mintcore is accidentally missing during deployment, the Cloud Function zip will lack mintcore files and fail at runtime with confusing import errors rather than at build time.

Info

[sub-agent-failure] The intent-coherence, style-conventions, and docs-currency sub-agents could not run (model claude-sonnet-4-5@20250929 unavailable on this deployment). These are sonnet-tier dimensions and do not block the review, but their findings are absent from this assessment.
[authentication-architecture] internal/mintcore/oidc.go — The JWKSVerifier implementation is architecturally sound: JWKS URI origin validation (SSRF prevention), RS256-only enforcement, proper claim validation (iss, aud, iat, exp, repository), kid-miss refresh rate limiting (30s cooldown), JWKS response size limiting (512KB), and 1-hour cache TTL.
[input-validation] internal/cli/admin.go:validateSkipMintCheck — Properly rejects embedded userinfo (credentials in URL), allows HTTP only for localhost/127.0.0.1/::1. storePEMToDisk validates role names using filepath.Base/filepath.Clean to prevent path traversal.
[authorization] internal/mintcore/github.go — The shared mintcore library faithfully preserves all security controls: cross-org installation verification, role-based permission downscoping via RolePermissions, and input validation patterns.

Previous run

Review

Findings

Medium

[data-exposure] internal/mint/main.go — The new /v1/status endpoint in the production Cloud Function is served before the Bearer token authentication check, returning allowedOrgs and all role-to-appID mappings to unauthenticated callers. While App IDs are publicly visible on GitHub, exposing the org-to-role mapping from a production endpoint without authentication is a defense-in-depth gap. The dev mint's unauthenticated /v1/status is acceptable given its development-only nature.
Remediation: Either gate /v1/status behind the same Bearer token check as /v1/token, or confirm that exposing this data unauthenticated is acceptable for the Cloud Function deployment. See also: [race-condition] finding on storePEMToDisk.
[race-condition] internal/cli/admin.go:storePEMToDisk — The function performs a non-atomic read-modify-write on config.json (read existing → unmarshal → modify → write temp → rename). If two concurrent calls race for different roles, one rename could overwrite the other's changes, losing a role entry. All current callers use sequential loops so this is a latent bug, not an active one, but the function has no internal synchronization and could be called concurrently in the future.
Remediation: Add file-level locking or document that storePEMToDisk must not be called concurrently for the same dataDir.

Low

[error-handling] internal/devmint/devmint.go:startPEMWatcher — The PEM watcher fires on any file change in the pems directory. Since storePEMToDisk writes the PEM file and config.json in separate operations, the watcher may fire between them, triggering a reload against partially-updated state. In practice, loadFromDisk reads config.json first (which hasn't been renamed yet), so it loads the old consistent state and then reloads again after the config rename. A small debounce (e.g., 100ms) would reduce unnecessary reload cycles.
[edge-case] internal/devmint/devmint.go:startPEMWatcher — The watcher only adds dataDir to the watch list if config.json already exists at startup. If the dev mint starts with an empty data directory, changes to config.json alone (e.g., manual edits) won't be detected until a PEM file also changes.
Remediation: Always add dataDir to the watcher regardless of whether config.json exists.
[test-inadequate] internal/mint/main_test.go — Several handler-level OIDC integration tests were removed (TestHandler_OIDCPrevalidation_, TestServeHTTP_PerRepo). The equivalent logic moved to internal/mintcore/ and is tested there, but handler-level integration coverage is thinner. The per-repo WIF provider tests and TestHandler_RoleAllowed positive test were removed without handler-level replacements.
[data-exposure] internal/devmint/devmint.go:/v1/status — When the dev mint runs with OIDC verification enabled (no --insecure-no-auth) and is exposed via --tunnel, the /v1/status endpoint is unauthenticated while /v1/token requires OIDC. Configuration data (org name, role names, app IDs) leaks through the public tunnel URL.

Info

[sub-agent-failure] The intent-coherence, style-conventions, docs-currency, and cross-repo-contracts sub-agents could not run (model claude-sonnet-4-5@20250929 unavailable on this deployment). These are sonnet-tier dimensions and do not block the review, but their findings are absent from this assessment.
[authentication-architecture] internal/mintcore/oidc.go — The replacement of GCP STS/WIF with direct JWKS-based OIDC verification is architecturally sound. The OIDCVerifier correctly fetches JWKS from the issuer's discovery endpoint, verifies RS256 signatures, checks issuer/audience/expiry/iat claims, and includes rate-limiting for kid-miss refreshes to prevent thundering herd. The origin check on jwks_uri prevents SSRF via a poisoned discovery document.
[input-validation] internal/cli/admin.go:validateSkipMintCheck — The new validation correctly rejects URLs with embedded userinfo (credentials in URL) and allows HTTP only for localhost/127.0.0.1/::1. Good defense-in-depth addition.

Previous run

Review

Findings

Medium

[data-exposure] internal/mint/main.go — The new /v1/status endpoint in the production Cloud Function is served before the Bearer token authentication check, returning allowedOrgs and all role-to-appID mappings to unauthenticated callers. While App IDs are publicly visible on GitHub, exposing the org-to-role mapping from a production endpoint without authentication is a defense-in-depth gap. The dev mint's unauthenticated /v1/status is acceptable given its development-only nature.
Remediation: Either gate /v1/status behind the same Bearer token check as /v1/token, or confirm that exposing this data unauthenticated is acceptable for the Cloud Function deployment. See also: [race-condition] finding on storePEMToDisk.
[race-condition] internal/cli/admin.go:storePEMToDisk — The function performs a non-atomic read-modify-write on config.json (read existing → unmarshal → modify → write temp → rename). If two concurrent calls race for different roles, one rename could overwrite the other's changes, losing a role entry. All current callers use sequential loops so this is a latent bug, not an active one, but the function has no internal synchronization and could be called concurrently in the future.
Remediation: Add file-level locking or document that storePEMToDisk must not be called concurrently for the same dataDir.

Low

[error-handling] internal/devmint/devmint.go:startPEMWatcher — The PEM watcher fires on any file change in the pems directory. Since storePEMToDisk writes the PEM file and config.json in separate operations, the watcher may fire between them, triggering a reload against partially-updated state. In practice, loadFromDisk reads config.json first (which hasn't been renamed yet), so it loads the old consistent state and then reloads again after the config rename. A small debounce (e.g., 100ms) would reduce unnecessary reload cycles.
[edge-case] internal/devmint/devmint.go:startPEMWatcher — The watcher only adds dataDir to the watch list if config.json already exists at startup. If the dev mint starts with an empty data directory, changes to config.json alone (e.g., manual edits) won't be detected until a PEM file also changes.
Remediation: Always add dataDir to the watcher regardless of whether config.json exists.
[test-inadequate] internal/mint/main_test.go — Several handler-level OIDC integration tests were removed (TestHandler_OIDCPrevalidation_, TestServeHTTP_PerRepo). The equivalent logic moved to internal/mintcore/ and is tested there, but handler-level integration coverage is thinner. The per-repo WIF provider tests and TestHandler_RoleAllowed positive test were removed without handler-level replacements.
[data-exposure] internal/devmint/devmint.go:/v1/status — When the dev mint runs with OIDC verification enabled (no --insecure-no-auth) and is exposed via --tunnel, the /v1/status endpoint is unauthenticated while /v1/token requires OIDC. Configuration data (org name, role names, app IDs) leaks through the public tunnel URL.

Info

[sub-agent-failure] The intent-coherence, style-conventions, docs-currency, and cross-repo-contracts sub-agents could not run (model claude-sonnet-4-5@20250929 unavailable on this deployment). These are sonnet-tier dimensions and do not block the review, but their findings are absent from this assessment.
[authentication-architecture] internal/mintcore/oidc.go — The replacement of GCP STS/WIF with direct JWKS-based OIDC verification is architecturally sound. The OIDCVerifier correctly fetches JWKS from the issuer's discovery endpoint, verifies RS256 signatures, checks issuer/audience/expiry/iat claims, and includes rate-limiting for kid-miss refreshes to prevent thundering herd. The origin check on jwks_uri prevents SSRF via a poisoned discovery document.
[input-validation] internal/cli/admin.go:validateSkipMintCheck — The new validation correctly rejects URLs with embedded userinfo (credentials in URL) and allows HTTP only for localhost/127.0.0.1/::1. Good defense-in-depth addition.

Previous run (2)

Review

Prior findings resolved

✅ [logic-error] internal/mintcore/claims.go (ValidateWorkflowRef) — The repository parameter has been added, restoring the cross-check that verifies the token’s repository claim matches the per-repo WIF repo key. The prior high-severity finding is resolved.

✅ [auth-bypass] internal/devmint/devmint.go — The --insecure-no-auth + --tunnel combination is now rejected at startup in mint_dev.go. Resolved.

✅ [missing-validation] internal/devmint/devmint.go (handleToken) — ValidateWorkflowRef is now called in the authenticated path. Request body size limiting, role/repo pattern validation, and repo count limits have been added. Both prior missing-validation findings are resolved.

✅ [stale-doc] docs/guides/infrastructure/infrastructure-reference.md — WIF/STS environment variables removed from documentation. Resolved.

✅ [stale-doc] docs/guides/getting-started/installation.md — --skip-mint-check description updated. Resolved.

✅ [incomplete-doc] CLAUDE.md — Mint sync instruction updated to mention mintcore. Resolved.

Findings

Medium

[non-atomic-write] internal/cli/admin.go (storePEMToDisk, ~line 411) — storePEMToDisk reads config.json, modifies the in-memory map, and writes it back without file locking. If two concurrent fullsend admin install processes run against the same --data-dir, one’s write can clobber the other’s. Additionally, a crash mid-write leaves a truncated config.json. Consider writing to a temp file and renaming atomically (os.Rename), or using os.O_CREATE|os.O_EXCL as a lock.
[silent-error] internal/cli/admin.go (storePEMToDisk, ~line 413) — When config.json exists but contains invalid JSON, json.Unmarshal fails and the error is silently discarded (the code proceeds with an empty map). This means a corrupted config file is silently overwritten rather than surfaced to the user. Consider returning the error or at minimum logging a warning.

Low

[defense-in-depth] internal/mintcore/oidc.go (discoverJWKSURI) — The JWKS URI origin validation has been added (prior finding addressed). The implementation correctly checks scheme+host match against the issuer. No action needed.
[edge-case] internal/mintcore/oidc.go (getKey) — A minRefreshInterval has been added to mitigate the thundering herd / unbounded JWKS fetch issue (prior finding addressed). The 30-second cooldown is reasonable for this use case.
[data-exposure] internal/devmint/devmint.go — Error messages in handleToken still include filesystem paths (e.g., PEM file locations). Low risk since the dev mint is a local development tool, but worth sanitizing if the tunnel feature sees wider use.
[path-traversal] internal/cli/admin.go (storePEMToDisk, ~line 405) — The role value from the server response is used directly in filepath.Join(dataDir, "pems", role+".pem"). A malicious mint server could return a role like ../../etc/crontab to write outside the data directory. Low severity because the dev mint is a local tool and the role values come from the user’s own config, but filepath.Clean + base-directory containment check would be defense-in-depth.

Previous run (3)

Review

Findings

High

[logic-error] internal/mintcore/claims.go (ValidateWorkflowRef) — Per-repo workflow ref validation lost a cross-check during refactoring. The original prevalidateOIDCToken derived repoKey from claims.Repository (the token’s own repository claim), ensuring only workflows FROM the registered per-repo repo could mint tokens. The refactored ValidateWorkflowRef iterates ALL perRepoWIFRepos keys and matches ANY against the job_workflow_ref prefix, without verifying that the token’s repository claim corresponds to the matched repo. This means a workflow in any org repo could call a reusable workflow hosted in a registered per-repo repo and successfully mint tokens that should only be available to that per-repo repo’s own workflows.
Remediation: Add a repository parameter to ValidateWorkflowRef (the token’s repository claim). In the per-repo branch, verify that the matched perRepoWIFRepos key matches strings.ToLower(repository) before accepting the match, replicating the original repoKey := strings.ToLower(claims.Repository) check.

Medium

[auth-bypass] internal/devmint/devmint.go — When --insecure-no-auth and --tunnel are used together, the dev mint accepts unauthenticated token requests over a public cloudflared URL. Anyone who discovers the tunnel URL can mint real GitHub App installation tokens for the configured org. There is no guard preventing this combination.
Remediation: When both --tunnel and --insecure-no-auth are specified, either refuse to start with a fatal error or require explicit confirmation. At minimum, print a prominent warning.
[missing-validation] internal/devmint/devmint.go (handleToken) — Even when OIDC verification is enabled (no --insecure-no-auth), the authenticated path calls ValidateOrgAllowed but never calls ValidateWorkflowRef. Any GitHub Actions workflow from the allowed org can request tokens, not just workflows from .fullsend, upstream, or registered per-repo repos. The production GCP mint calls both. This is a security regression in the dev mint’s authenticated path.
Remediation: Add mintcore.ValidateWorkflowRef(claims.JobWorkflowRef, ...) after the org validation check. The Options struct should accept allowed workflow files and per-repo WIF repos configuration.
[missing-validation] internal/devmint/devmint.go (handleToken) — The dev mint does not validate request body size, role format (mintcore.RolePattern), repo name format (mintcore.RepoNamePattern), repo count limit, or .. traversal in repo names. The production mint enforces all of these. An unsanitized repo name containing / could alter the GitHub API request path in FindInstallation.
Remediation: Add role pattern validation, repo name pattern validation, body size limit (io.LimitReader), and repo count limit to match the production mint.
[stale-doc] docs/guides/infrastructure/infrastructure-reference.md:270-271 — Documents WIF_POOL_NAME and WIF_PROVIDER_NAME as Cloud Function environment variables, but this PR removes them from requiredEnvVars and deletes all WIF/STS code. Users following this guide would configure unnecessary variables.
Remediation: Remove WIF_POOL_NAME and WIF_PROVIDER_NAME from the env vars list in the Cloud Function deployment diagram.
[stale-doc] docs/guides/getting-started/installation.md:233,238 — The --skip-mint-check description says it skips "app setup" and "only validates that the URL uses HTTPS." Both claims are now stale: this PR changes --skip-mint-check to allow app setup (the condition !skipAppSetup && !skipMintCheck was changed to !skipAppSetup), and validateSkipMintCheck now permits HTTP for localhost/127.0.0.1/::1.
Remediation: Update the flag description to reflect that it now skips only GCP mint provisioning (not app setup), and note the HTTP-for-localhost exception.
[incomplete-doc] CLAUDE.md — The mint sync instruction says "When changing internal/mint/main.go, always copy it to internal/dispatch/gcf/mintsrc/main.go.embed." With the introduction of internal/mintcore/, this instruction is incomplete — changes to mintcore files also need to be copied to mintsrc/mintcore/*.embed files.
Remediation: Update the CLAUDE.md mint sync section to mention the mintcore module and its .embed counterparts.

Low

[defense-in-depth] internal/mintcore/oidc.go (discoverJWKSURI) — The JWKS URI returned by the OIDC discovery document is not validated to share the same origin as the issuer URL. While the issuer is hardcoded to https://token.actions.githubusercontent.com (making exploitation impractical), validating the JWKS URI origin is a defense-in-depth best practice.
[edge-case] internal/mintcore/oidc.go (getKey) — Multiple concurrent goroutines can discover a missing/expired key simultaneously and all call refreshKeys, causing redundant JWKS fetches (thundering herd). An attacker sending tokens with random kid values could also trigger unbounded JWKS fetches. Consider adding a minimum refresh interval or sync.Once-style deduplication.
[data-exposure] internal/mint/main.go (handleStatus) — The /v1/status endpoint is unauthenticated and returns allowed orgs and role-to-appID mappings. While this data is arguably public (app IDs are visible on GitHub), exposing the mint’s internal configuration aids reconnaissance.
[edge-case] internal/devmint/devmint.go (loadFromDisk) — loadFromDisk appends to existing pems and appIDs maps but never clears them. During hot-reload, if a role is removed from config.json, the stale role’s PEM remains in memory and can still be used to mint tokens.
[data-exposure] internal/devmint/devmint.go (handleToken) — Error messages reveal filesystem paths (e.g., "write PEM to /home/user/.fullsend-dev-mint/pems/coder.pem") which may leak internal state when the server is exposed via tunnel.

Previous run (4)

Review

Findings

Medium

[docs-currency] docs/guides/infrastructure/dev-mint.md:170-172 — The POST /v1/token API reference example shows repos in org-prefixed format ("repos": ["myorg/some-repo"]), but the dev mint code passes req.Repos[0] directly to findInstallation(ctx, ..., org, repo) where org is already the stored server-side org name. This constructs the GitHub API URL as /repos/{org}/{repo}/installation — so an org-prefixed repo like "myorg/some-repo" would produce /repos/myorg/myorg/some-repo/installation, which 404s. The production mint (internal/mint/main.go) explicitly rejects org-prefixed repos via repoNamePattern. Users following this API reference would get errors.
Remediation: Change the example to use bare repo names: "repos": ["some-repo"].

Info

[correctness] internal/cli/admin.go (postPEMToMint) — io.ReadAll(resp.Body) on error responses has no size limit. Low risk since this is a CLI calling a local dev server, but inconsistent with the bounded reads in internal/devmint/github.go which uses io.LimitReader.

Previous run (5)

Review

Findings

Medium

[docs-currency] docs/guides/getting-started/installation.md:233,238 — The --skip-mint-check flag description says it skips "app setup" and "only validates that the URL uses HTTPS." Both claims are now stale: this PR changes --skip-mint-check to allow app setup (apps are created and PEMs are POSTed to the dev mint), and validateSkipMintCheck now permits HTTP for localhost/127.0.0.1/::1. Users following the installation guide would be misled about what the flag does.
Remediation: Update the --skip-mint-check description in installation.md to reflect that it now skips only GCP mint provisioning (not app setup), and note the HTTP-for-localhost exception.

Low

[correctness] internal/devmint/github.go:20-30 — The rolePermissions map duplicates the canonical map in internal/mint/main.go:773-781. These must stay in sync but share no common source of truth. A permission change in one file without the other would silently produce tokens with wrong scopes.
Remediation: Consider extracting the shared permission definitions to a common package, or add a test that compares both maps.
[docs-currency] docs/guides/infrastructure/dev-mint.md:29 — The prerequisites reference "the feat/dev-mint branch" which will be stale once this PR merges to main.
Remediation: Change to reference a version/release or remove the branch reference.

Info

[correctness] internal/cli/admin.go (postPEMToMint) — io.ReadAll(resp.Body) on error responses has no size limit. Low risk since this is a CLI calling a local dev server, but inconsistent with the bounded reads in internal/devmint/github.go which uses io.LimitReader.

Previous run (6)

Review

Findings

High

[logic-error] internal/mintcore/claims.go (ValidateWorkflowRef) — Per-repo workflow ref validation lost a cross-check during refactoring. The original prevalidateOIDCToken derived repoKey from claims.Repository (the token's own repository claim), ensuring only workflows FROM the registered per-repo repo could mint tokens. The refactored ValidateWorkflowRef iterates ALL perRepoWIFRepos keys and matches ANY against the job_workflow_ref prefix, without verifying that the token's repository claim corresponds to the matched repo. This means a workflow in any org repo could call a reusable workflow hosted in a registered per-repo repo and successfully mint tokens that should only be available to that per-repo repo's own workflows.
Remediation: Add a repository parameter to ValidateWorkflowRef (the token's repository claim). In the per-repo branch, verify that the matched perRepoWIFRepos key matches strings.ToLower(repository) before accepting the match, replicating the original repoKey := strings.ToLower(claims.Repository) check.

Medium

[auth-bypass] internal/devmint/devmint.go — When --insecure-no-auth and --tunnel are used together, the dev mint accepts unauthenticated token requests over a public cloudflared URL. Anyone who discovers the tunnel URL can mint real GitHub App installation tokens for the configured org. There is no guard preventing this combination.
Remediation: When both --tunnel and --insecure-no-auth are specified, either refuse to start with a fatal error or require explicit confirmation. At minimum, print a prominent warning.
[missing-validation] internal/devmint/devmint.go (handleToken) — Even when OIDC verification is enabled (no --insecure-no-auth), the authenticated path calls ValidateOrgAllowed but never calls ValidateWorkflowRef. Any GitHub Actions workflow from the allowed org can request tokens, not just workflows from .fullsend, upstream, or registered per-repo repos. The production GCP mint calls both. This is a security regression in the dev mint's authenticated path.
Remediation: Add mintcore.ValidateWorkflowRef(claims.JobWorkflowRef, ...) after the org validation check. The Options struct should accept allowed workflow files and per-repo WIF repos configuration.
[missing-validation] internal/devmint/devmint.go (handleToken) — The dev mint does not validate request body size, role format (mintcore.RolePattern), repo name format (mintcore.RepoNamePattern), repo count limit, or .. traversal in repo names. The production mint enforces all of these. An unsanitized repo name containing / could alter the GitHub API request path in FindInstallation.
Remediation: Add role pattern validation, repo name pattern validation, body size limit (io.LimitReader), and repo count limit to match the production mint.
[stale-doc] docs/guides/infrastructure/infrastructure-reference.md:270-271 — Documents WIF_POOL_NAME and WIF_PROVIDER_NAME as Cloud Function environment variables, but this PR removes them from requiredEnvVars and deletes all WIF/STS code. Users following this guide would configure unnecessary variables.
Remediation: Remove WIF_POOL_NAME and WIF_PROVIDER_NAME from the env vars list in the Cloud Function deployment diagram.
[stale-doc] docs/guides/getting-started/installation.md:233,238 — The --skip-mint-check description says it skips "app setup" and "only validates that the URL uses HTTPS." Both claims are now stale: this PR changes --skip-mint-check to allow app setup (the condition !skipAppSetup && !skipMintCheck was changed to !skipAppSetup), and validateSkipMintCheck now permits HTTP for localhost/127.0.0.1/::1.
Remediation: Update the flag description to reflect that it now skips only GCP mint provisioning (not app setup), and note the HTTP-for-localhost exception.
[incomplete-doc] CLAUDE.md — The mint sync instruction says "When changing internal/mint/main.go, always copy it to internal/dispatch/gcf/mintsrc/main.go.embed." With the introduction of internal/mintcore/, this instruction is incomplete — changes to mintcore files also need to be copied to mintsrc/mintcore/*.embed files.
Remediation: Update the CLAUDE.md mint sync section to mention the mintcore module and its .embed counterparts.

Low

[defense-in-depth] internal/mintcore/oidc.go (discoverJWKSURI) — The JWKS URI returned by the OIDC discovery document is not validated to share the same origin as the issuer URL. While the issuer is hardcoded to https://token.actions.githubusercontent.com (making exploitation impractical), validating the JWKS URI origin is a defense-in-depth best practice.
[edge-case] internal/mintcore/oidc.go (getKey) — Multiple concurrent goroutines can discover a missing/expired key simultaneously and all call refreshKeys, causing redundant JWKS fetches (thundering herd). An attacker sending tokens with random kid values could also trigger unbounded JWKS fetches. Consider adding a minimum refresh interval or sync.Once-style deduplication.
[data-exposure] internal/mint/main.go (handleStatus) — The /v1/status endpoint is unauthenticated and returns allowed orgs and role-to-appID mappings. While this data is arguably public (app IDs are visible on GitHub), exposing the mint's internal configuration aids reconnaissance.
[edge-case] internal/devmint/devmint.go (loadFromDisk) — loadFromDisk appends to existing pems and appIDs maps but never clears them. During hot-reload, if a role is removed from config.json, the stale role's PEM remains in memory and can still be used to mint tokens.
[data-exposure] internal/devmint/devmint.go (handleToken) — Error messages reveal filesystem paths (e.g., "write PEM to /home/user/.fullsend-dev-mint/pems/coder.pem") which may leak internal state when the server is exposed via tunnel.

Previous run (7)

Review

Findings

Medium

[docs-currency] docs/guides/infrastructure/dev-mint.md:170-172 — The POST /v1/token API reference example shows repos in org-prefixed format ("repos": ["myorg/some-repo"]), but the dev mint code passes req.Repos[0] directly to findInstallation(ctx, ..., org, repo) where org is already the stored server-side org name. This constructs the GitHub API URL as /repos/{org}/{repo}/installation — so an org-prefixed repo like "myorg/some-repo" would produce /repos/myorg/myorg/some-repo/installation, which 404s. The production mint (internal/mint/main.go) explicitly rejects org-prefixed repos via repoNamePattern. Users following this API reference would get errors.
Remediation: Change the example to use bare repo names: "repos": ["some-repo"].

Info

[correctness] internal/cli/admin.go (postPEMToMint) — io.ReadAll(resp.Body) on error responses has no size limit. Low risk since this is a CLI calling a local dev server, but inconsistent with the bounded reads in internal/devmint/github.go which uses io.LimitReader.

Previous run (8)

Review

Findings

Medium

[docs-currency] docs/guides/getting-started/installation.md:233,238 — The --skip-mint-check flag description says it skips "app setup" and "only validates that the URL uses HTTPS." Both claims are now stale: this PR changes --skip-mint-check to allow app setup (apps are created and PEMs are POSTed to the dev mint), and validateSkipMintCheck now permits HTTP for localhost/127.0.0.1/::1. Users following the installation guide would be misled about what the flag does.
Remediation: Update the --skip-mint-check description in installation.md to reflect that it now skips only GCP mint provisioning (not app setup), and note the HTTP-for-localhost exception.

Low

[correctness] internal/devmint/github.go:20-30 — The rolePermissions map duplicates the canonical map in internal/mint/main.go:773-781. These must stay in sync but share no common source of truth. A permission change in one file without the other would silently produce tokens with wrong scopes.
Remediation: Consider extracting the shared permission definitions to a common package, or add a test that compares both maps.
[docs-currency] docs/guides/infrastructure/dev-mint.md:29 — The prerequisites reference "the feat/dev-mint branch" which will be stale once this PR merges to main.
Remediation: Change to reference a version/release or remove the branch reference.

Info

[correctness] internal/cli/admin.go (postPEMToMint) — io.ReadAll(resp.Body) on error responses has no size limit. Low risk since this is a CLI calling a local dev server, but inconsistent with the bounded reads in internal/devmint/github.go which uses io.LimitReader.

ifireball

The PR is a missed opportunity. It goes part of the way towards implementing a GCP-independent mint that could e.g. run in a container on k8s but stops short because of "dev" focus and then adds some "dev" APIs that seem redundant.

The following approach would be welcome instead, and would actually also be a step towards the public mint deployment:

Implement GCP-idependent OIDC token verification via the keys in token.actions.githubusercontent.com
Drop the POST /v2/pem API (it doesn't seem beneficial to have writable mints)
Add the other APIs to the GCP mint implementation
Try to share as much code as possible between the mint implementations, place pieces that must be different behind interfactes that can abstract away multiple implementations.

ggallen · 2026-06-01T17:36:10Z

@ifireball, let me see what I can do.

ggallen · 2026-06-02T01:33:09Z

@ifireball, is this more to your liking? Have at it!

fullsend-ai-review

See the review comment for full details.

ifireball · 2026-06-02T07:30:11Z

@ifireball, is this more to your liking? Have at it!

yes, thanks! a few small nits:

The GCP function should probably keep the STS-based OIDC verification by default, but the option to do local verification would be handy if we want to port to other clouds
At this point, is this really a "dev" mint or simply a form-factor that can run stand-alone and read PEMs from files? Perhaps the command should become fullsend mint run ?
Hot-reloading the PEMs might be a bit of an overkill, can me make do with static loading?

Also about the code structure - I was expecting to see:

Multiple OIDC verifiers (STS, local) behind a common interface
Multiple PEM loaders (GCP secret manager, files) behind a common interface
A common mint core that takes the providers and exposes some kind of an HTTP handler interface so it can be mounted either as a GCP function or on a stand-alone server.

Not quite seeing that structure in the code ATM.

Having said all that, the size of this change scares me a bit, how do we know we did not break the original mint?

ggallen · 2026-06-02T11:58:33Z

Having said all that, the size of this change scares me a bit, how do we know we did not break the original mint?
I actually think we would find that relatively quickly. I assume nothing would work.

ifireball · 2026-06-02T12:29:37Z

Having said all that, the size of this change scares me a bit, how do we know we did not break the original mint?
I actually think we would find that relatively quickly. I assume nothing would work.

Either that, or everything will become silently insecure because we've dropped some token validations without noticing...

Introduce a standalone token mint (fullsend mint run) that replaces the full GCP infrastructure for local development and evaluation. The mint stores GitHub App PEMs on disk, verifies OIDC tokens via JWKS, and mints real installation tokens. Refactor both mints to share code via internal/mintcore/: - OIDCVerifier interface with JWKSVerifier (standalone) and STSVerifier (GCP) - PEMAccessor interface for PEM storage abstraction - Shared claims validation, GitHub API helpers, and role permissions - BuildRepoProviderID for WIF provider resolution GCP mint keeps STS-based OIDC verification as default. The /v1/status endpoint now requires authentication in the GCP mint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ggallen · 2026-06-02T13:48:01Z

@ifireball Thanks for the thorough review — all five points addressed:

Keep STS in GCP mint — Restored STSVerifier as the default OIDC verifier for the Cloud Function. WIF_POOL_NAME and WIF_PROVIDER_NAME are back in requiredEnvVars. The standalone mint uses JWKSVerifier (direct JWKS verification), keeping each mint on its battle-tested path.
Rename dev → run — Command is now fullsend mint run. File renamed from mint_dev.go to mint_run.go, env var is FULLSEND_MINT_RUN_DATA_DIR, docs updated throughout.
Remove fsnotify — Hot-reload removed entirely. PEMs load once at startup from disk; restart to pick up changes. fsnotify dependency dropped from go.mod.
Restructure behind interfaces —
- OIDCVerifier interface in mintcore with two implementations: JWKSVerifier (standalone) and STSVerifier (GCP)
- PEMAccessor interface moved from internal/mint/main.go to internal/mintcore/pem.go
- BuildRepoProviderID extracted to internal/mintcore/wif.go (was duplicated between mint and provisioner)
- Both mints use mintcore.OIDCVerifier (interface type) for their verifier field
Don't break the original mint — GCP mint's ServeHTTP flow is structurally close to upstream. STS verification, WIF provider resolution, org/workflow validation all restored. The /v1/status endpoint now requires authentication in the GCP mint (was flagged by the review bot as a data-exposure gap).

Also fixed the review bot finding about unauthenticated /v1/status in the Cloud Function, registered the new mintcore embed files (sts.go, wif.go, pem.go) in the provisioner's embeddedMintFiles map and go:embed directive, and updated infrastructure-reference.md to include the WIF env vars in the Cloud Function diagram.

All tests pass (mintcore, mint, devmint, dispatch/gcf, full go test ./...).

ggallen · 2026-06-02T14:25:01Z

This has been split into #1783 and #1784.

…dpoint Resolves fullsend-ai#1751 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

Extract shared OIDC verification, STS token exchange, claims validation, and GitHub API helpers from internal/mint into a new internal/mintcore Go module. Introduces OIDCVerifier interface with STSVerifier (GCP WIF) and JWKSVerifier (direct JWKS) implementations, authenticated /v1/status endpoint, and immutable RolePermissions accessor. Intentional behavioral change: /v1/token OIDC auth failures now return HTTP 401 (was 403) for correct HTTP semantics and consistency with /v1/status. Authorization failures (role not allowed) remain 403. Resolves the first half of fullsend-ai#1751. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

ifireball · 2026-06-08T08:39:38Z

@ggallen I'm a little confused, is this PR still relevant? I thought we already merged the mintcore refactor

ggallen · 2026-06-08T17:37:12Z

Superseded by #1783.

fullsend-ai-retro · 2026-06-08T17:38:50Z

🤖 Retro · Started 5:38 PM UTC
Commit: d0ac11b · View workflow run →

fullsend-ai-retro · 2026-06-08T17:47:47Z

Retro: PR #1751 — mintcore shared library and dev mint

Workflow: Human-authored PR by ggallen (46 files, +4517/−2188). 8 review agent runs dispatched (6 completed, 2 cancelled). Human reviewer (ifireball) requested architectural changes. PR ultimately closed and split into #1783 and #1784.

What went well:

The review bot caught a genuine HIGH severity security bug — ValidateWorkflowRef lost its cross-repo check during refactoring, which would have allowed any org workflow to mint tokens for registered per-repo repos. This is exactly the kind of finding that justifies automated review.
The bot also caught auth-bypass issues (--insecure-no-auth + --tunnel combination) and missing validation in the dev mint.
The author addressed all bot findings across iterations.

What could improve:

The PR was too large for effective review. The human reviewer explicitly flagged this ("the size of this change scares me"). The bot never raised this concern — it processed all 46 files without suggesting decomposition.
8 review runs consumed ~74 minutes of agent compute. Existing issues (Debounce review dispatch on rapid synchronize events #1014, Add concurrency group to dispatch-review job in fullsend.yaml #981, Reduce wasted review runs from stale-head races #1331, Review agent accumulates redundant internal iterations on clean diffs #1370) already address rapid-fire dispatch and redundant reviews.
The architectural feedback (use interfaces, rename "dev" → "run", drop fsnotify) came only from the human. Existing issues Add 'strategic fitness' evaluation dimension to review agent #849 and Review agent should assess feature-level design, not just implementation #1469 track the design-level review gap.

1 proposal filed (large PR detection). Several potential proposals were skipped because existing issues already cover them:

Proposals filed

Review agent should flag large PRs and recommend splitting (in fullsend-ai/fullsend)

fullsend-ai-retro · 2026-06-08T17:47:48Z

🤖 Finished Retro · ✅ Success · Started 5:38 PM UTC · Completed 5:47 PM UTC
Commit: d0ac11b · View workflow run →

github-actions Bot deployed to site-preview June 1, 2026 15:27 View deployment

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 1, 2026

ifireball requested changes Jun 1, 2026

View reviewed changes

ggallen force-pushed the feat/dev-mint branch from a8421d7 to bc43d33 Compare June 1, 2026 17:30

github-actions Bot deployed to site-preview June 1, 2026 17:32 View deployment

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

ggallen changed the title ~~feat: add development-only token mint for evaluation~~ feat: add mintcore shared library and dev mint for local evaluation Jun 2, 2026

ggallen force-pushed the feat/dev-mint branch 2 times, most recently from 6d4f2cf to 1bc975d Compare June 2, 2026 01:19

github-actions Bot deployed to site-preview June 2, 2026 01:21 View deployment

ggallen force-pushed the feat/dev-mint branch from 1bc975d to f63adf3 Compare June 2, 2026 01:28

github-actions Bot deployed to site-preview June 2, 2026 01:29 View deployment

fullsend-ai-review Bot suggested changes Jun 2, 2026

View reviewed changes

Comment thread internal/mintcore/claims.go

Comment thread internal/devmint/devmint.go

Comment thread internal/devmint/devmint.go

Comment thread internal/devmint/devmint.go

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

ggallen force-pushed the feat/dev-mint branch from f63adf3 to a61a31e Compare June 2, 2026 01:53

github-actions Bot deployed to site-preview June 2, 2026 01:54 View deployment

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 2, 2026

ggallen force-pushed the feat/dev-mint branch from a61a31e to 08d82b9 Compare June 2, 2026 02:22

github-actions Bot deployed to site-preview June 2, 2026 02:23 View deployment

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

ggallen force-pushed the feat/dev-mint branch from 08d82b9 to 0413486 Compare June 2, 2026 13:44

github-actions Bot deployed to site-preview June 2, 2026 13:46 View deployment

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

ggallen mentioned this pull request Jun 2, 2026

feat: extract mintcore shared library with JWKSVerifier and status endpoint #1783

Merged

4 tasks

ifireball assigned ggallen Jun 4, 2026

fullsend-ai-triage Bot mentioned this pull request Jun 5, 2026

Standalone dev mint server (no GCP required) #1963

Open

5 tasks

ggallen closed this Jun 8, 2026

fullsend-ai-retro Bot mentioned this pull request Jun 8, 2026

Review agent should flag large PRs and recommend splitting #2034

Open

github-actions Bot deleted the feat/dev-mint branch July 5, 2026 06:31

Uh oh!

Conversation

ggallen commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

mintcore shared library (internal/mintcore/)

GCP mint refactoring (internal/mint/)

Dev mint (internal/devmint/)

CLI changes

Cloud Function deployment

Changes from review feedback

Test plan

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

fullsend-ai-review Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Prior findings resolved

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Prior findings resolved

Findings

Medium

Low

Review

Findings

High

Medium

Low

Review

Findings

Medium

Info

Review

Findings

Medium

Low

Info

Review

Findings

High

Medium

Low

Review

Findings

Medium

Info

Review

Findings

Medium

Low

Info

Uh oh!

ifireball left a comment

Choose a reason for hiding this comment

Uh oh!

ggallen commented Jun 1, 2026

Uh oh!

ggallen commented Jun 2, 2026

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ggallen commented Jun 1, 2026 •

edited

Loading

mintcore shared library (`internal/mintcore/`)

GCP mint refactoring (`internal/mint/`)

Dev mint (`internal/devmint/`)

github-actions Bot commented Jun 1, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 1, 2026 •

edited

Loading