feat: extract mintcore shared library with JWKSVerifier and status endpoint by ggallen · Pull Request #1783 · fullsend-ai/fullsend

ggallen · 2026-06-02T14:20:30Z

Summary

Extract shared types and functions from internal/mint/main.go into a new internal/mintcore/ Go module, then refactor the GCP mint to import from it. This PR also introduces new features alongside the refactoring.

This is the first half of #1751, split to isolate the production-risk changes for focused review.

New features

JWKSVerifier: Direct JWKS-based OIDC token verification (for future dev mint and non-GCP deployments)
/v1/status endpoint: Authenticated endpoint returning configured orgs and roles
RolePermissions immutability: RolePermissions is now a function returning a deep copy, preventing accidental mutation of the canonical permission map

What moved to mintcore

OIDCVerifier interface with JWKSVerifier (new) and STSVerifier (extracted from GCP mint)
Claims/Audience types and validation (ValidateOrgAllowed, ValidateWorkflowRef)
GitHub API helpers (GenerateAppJWT, FindInstallation, CreateInstallationToken)
Pattern constants (GitHubOrgPattern, RepoNamePattern, RolePattern)
PEMAccessor interface for pluggable PEM storage
WIF helper (BuildRepoProviderID)

What changed in the GCP mint

main.go now imports from mintcore instead of defining types/functions inline
STSVerifier replaces inline stsTokenValidator (same logic, just relocated)
NewHandler constructs the verifier internally
/v1/status endpoint added for mint health inspection (authenticated)

Provisioner / Cloud Function deployment

Provisioner bundles mintcore/ source alongside the mint function in the deployment zip
go.mod replace directive rewritten from ../mintcore to ./mintcore during zip creation
9 new .embed files for mintcore (synced from source, including go.sum)
Embed sync test extended to cover mintcore files

Review focus

Is the STSVerifier extraction faithful — same logic, no behavioral changes?
Does bundleFunctionSource correctly nest mintcore in the deployment zip?
Are all embedded files in sync with their source counterparts?
Does JWKSVerifier correctly validate org allowlist and workflow refs?

Test plan

go test ./internal/mintcore/... — all tests pass (including new JWKSVerifier validation tests)
go test ./internal/mint/... — all tests pass (including new STSVerifier integration test)
go test ./internal/dispatch/gcf/... — provisioner tests pass (including embed sync)
go vet ./... — clean

🤖 Generated with Claude Code

github-actions · 2026-06-02T14:22:13Z

Site preview

Preview: https://60b304bd-site.fullsend-ai.workers.dev

Commit: 51a084cb3c535d2c383cd3d8f9cdf39dfc25d5d4

fullsend-ai-review · 2026-06-02T14:34:23Z

Review — PR #1783

Outcome: ✅ Approve

Change summary: Extracts shared OIDC verification, STS token exchange, claims validation, and GitHub API helpers from internal/mint into a new internal/mintcore package. Updates Cloud Function bundling to include the new package. Adds /v1/status endpoint. Comprehensive test refactoring replaces fake validators with real JWKS-based test infrastructure.

Resolved prior findings

[logic-error] discoverJWKSURI issuer validation — Resolved. The new code validates doc.Issuer != v.issuerURL before using the discovered JWKS URI, preventing issuer-mismatch attacks.
[edge-case] getKey write lock on hot path — Resolved. Now uses RLock() for cache hits with write lock only on cache miss, eliminating unnecessary contention.

Findings

#	Severity	Category	File	Description
1	low	data-exposure	`internal/mint/main.go`	`/v1/status` endpoint returns available role names for the caller's org. While gated behind OIDC verification, this reveals internal configuration. Consider whether role enumeration is acceptable for all authenticated callers.
2	low	error-handling	`internal/dispatch/gcf/provisioner.go`	`addDirToZip` detects symlinks for files via `Lstat` but does not check whether subdirectories are symlinks before recursing into them. Low risk in practice since the source tree is controlled.
3	low	test-coverage-reduction	`internal/mint/main_test.go`	Handler-level tests were significantly refactored. New mintcore unit tests (`claims_test.go`, `oidc_test.go`, `sts_test.go`) and new integration-style handler tests cover the extracted logic well. Verify that edge cases from removed `fakeTokenValidator` tests (e.g., malformed tokens, expired claims) are covered by the new `newTestOIDCEnv` infrastructure.
4	info	sub-agent-failure	N/A	The intent-coherence, style-conventions, and docs-currency sub-agents failed to run due to model unavailability (`claude-sonnet-4-5@20250929 not available on vertex deployment`). These are sonnet-tier dimensions; correctness and security (opus-tier) completed successfully.

Notes

The correctness sub-agent raised a HIGH finding about handleStatus role extraction potentially producing incorrect prefix slicing when strings.ToLower(org) changes byte length. This is a false positive: GitHubOrgPattern constrains org names to ASCII [a-zA-Z0-9-], and ASCII lowercasing always preserves byte length. Discarded during challenger pass.
The security sub-agent returned no findings — the OIDC/STS extraction preserves all existing security properties and the new /v1/status endpoint is properly gated.
Prior review provenance: app-verified. Severity anchoring applied to unchanged code.

Previous run

Prior review evaluated commit 4c2e750.

Previous run

Review

Findings

Medium

[logic-error] internal/mintcore/oidc.go:315 — discoverJWKSURI parses the issuer field from the OIDC discovery document (discoveryDoc.Issuer) but never validates that it matches v.issuerURL. The OIDC Discovery spec (Section 4.3) requires the consumer to verify issuer == expected_issuer before trusting the document. The existing origin check (scheme+host match on jwks_uri) provides partial mitigation but does not satisfy the spec requirement.
Remediation: Add if doc.Issuer != v.issuerURL { return "", fmt.Errorf("issuer mismatch: got %q, want %q", doc.Issuer, v.issuerURL) } after unmarshalling the discovery document. Apply the same fix to the .embed copy.
[test-coverage-reduction] internal/mint/main_test.go — Several handler-level integration tests were removed without full equivalent replacements at the handler integration level: TestHandler_OIDCPrevalidation_UpstreamWorkflowRef, TestHandler_OIDCPrevalidation_PerRepoCrossRepoRef, TestHandler_OIDCPrevalidation_PerRepoUnregistered, TestHandler_OIDCPrevalidation_PerRepoMixedCase, TestHandler_OIDCPrevalidation_NonWorkflowPath, and TestServeHTTP_PerRepoProvider. Unit-level coverage exists via TestValidateWorkflowRef and TestHandler_PerRepoWIF_RestrictedWorkflows, but the end-to-end handler flow for these scenarios (request → auth → validation → provider selection) is no longer exercised.
Remediation: Add handler-level integration tests that exercise the full request flow for upstream workflow ref routing and per-repo WIF provider selection through the refactored handler.

Low

[data-exposure] internal/mint/main.go:291 — /v1/status returns configured role names to any authenticated GitHub Actions workflow. The endpoint requires authentication (OIDC token validation) and the code comments document this as intentional (non-secret deployment config). Low risk, but role names could reveal internal infrastructure naming conventions to any workflow in any org with a valid OIDC token.
[edge-case] internal/mintcore/oidc.go:237 — getKey acquires a write lock (v.mu.Lock()) even on the hot path where cached keys are found. A read-lock-first pattern with promotion to write lock on cache miss would reduce contention under concurrent verification load.
[error-handling] internal/dispatch/gcf/provisioner.go:1730 — addDirToZip uses filepath.Walk without guarding against symlink cycles or deeply nested directory structures. The filepath.Walk does follow symlinks by default; while the deployment directory is controlled, a defensive filepath.WalkDir with symlink detection would be more robust.

Info

[sub-agent-failure] N/A — Four sonnet-tier sub-agents (intent-coherence, style-conventions, docs-currency, cross-repo-contracts) failed to return findings due to model unavailability (claude-sonnet-4-5@20250929 not available on vertex deployment). These dimensions were not evaluated in this review.

Previous run (2)

Review

Findings

Medium

[test-coverage-reduction] internal/mint/main_test.go — Several handler-level integration tests were removed during the mintcore extraction without equivalent handler-level replacements. Tests for AudienceUnmarshalJSON (8 sub-tests), CheckAllowedOrg (10 cases), PrevalidateOIDCToken_InvalidFormat, Handler_OIDCPrevalidation_WorkflowAllowlist, and BuildRepoProviderID (cross-module sync test) were removed from the handler test suite. While equivalent unit tests exist in internal/mintcore/*_test.go, the handler-level integration coverage is now thinner — regressions in how the handler wires the mintcore library would go undetected. Consider adding a small set of integration tests that exercise the handler→mintcore boundary for key validation paths.
[missing-doc] docs/guides/infrastructure/infrastructure-reference.md — The new /v1/status endpoint (authenticated GET returning configured org and roles) is not documented in the infrastructure reference guide. This endpoint is a new public interface on the mint Cloud Function that clients may depend on for discovering available roles. Add documentation covering: endpoint path (GET /v1/status), authentication requirement (Bearer OIDC JWT), response format ({org, roles[]}), and use case (discover available roles for the authenticated org).

Low

[logic-error] internal/mint/main.go — The comment "Auth check runs before method check intentionally: unauthenticated callers get 401 for all paths, preventing endpoint enumeration" contradicts the actual routing order. A POST /v1/status without authentication returns 405 (method not allowed) before reaching the auth check, while GET /v1/status without auth returns 401. This difference reveals endpoint method constraints to unauthenticated callers. The practical impact is minimal since endpoint existence is already revealed by GET → 401, but the comment should either be corrected to match the code or the routing reordered to match the stated intent. Same issue exists in .embed copy.
[doc-style] internal/mint/main.go — Documentation comments were removed from unexported types mintError and function writeError during the refactoring. The existing codebase convention includes comments on unexported identifiers (see smPEMAccessor, stsTokenValidator in the same file). Consider restoring brief comments for consistency.
[data-exposure] internal/mint/main.go — The /v1/status endpoint returns configured role names (e.g., triage, coder, review) to any authenticated workflow in the org. While authentication is required and only the caller's org roles are returned, role names provide reconnaissance value to an attacker who has compromised a workflow token. The exposure is limited and the test correctly verifies that app IDs and org-scoped keys are not leaked.

Previous run (3)

Review

Findings

Medium

[data-exposure] internal/mint/main.go:handleStatus — The /v1/status endpoint returns the caller's org name and a filtered list of roles available for that org to any caller with a valid OIDC token from any allowed org. This is improved from the prior review: the response now returns only the requesting caller's single org (derived from the OIDC token's repository_owner claim) rather than exposing all allowedOrgs, and roles are org-stripped names only. However, no role-based authorization is enforced — any workflow from any allowed org (including triage-level) can enumerate available roles for that org, aiding reconnaissance. See also: [authorization] finding on this endpoint.
Remediation: Restrict the endpoint to callers with a specific administrative role, or return only the roles the caller's workflow is authorized to use.

Low

[authorization] internal/mint/main.go — The /v1/status endpoint performs OIDC verification but does not check whether the authenticated caller has any specific role or elevated privilege. Any workflow from an allowed org with a valid OIDC token can query the endpoint. See also: [data-exposure] finding on this endpoint.
[logic-error] internal/mint/main.go:ServeHTTP — The auth header check now runs before the POST method check for /v1/token and / paths. This means GET /v1/token without auth returns 401 instead of the previous 405. Arguably more secure (avoids leaking endpoint existence to unauthenticated callers) but is an undocumented behavior change.
[edge-case] internal/dispatch/gcf/provisioner.go:addDirToZip — The function skips directories, so if internal/mintcore/ ever gains subdirectories, those files would be silently excluded from the Cloud Function deployment zip. Fine today since mintcore is flat.
[edge-case] internal/mintcore/oidc.go:refreshKeys — After the first successful JWKS discovery, the cachedJWKSURI is never invalidated. Changes to the OIDC discovery document's jwks_uri (e.g., key infrastructure migration) will never be picked up — the mint process must be restarted. This is a deliberate design tradeoff (avoids two HTTP requests per refresh).
[race-condition] internal/mintcore/oidc.go:getKey — Between refreshGroup.Do returning and the subsequent v.mu.Lock(), another goroutine could trigger a new refresh cycle. The consequence is a slightly stale lastKidMissAt timestamp — a performance throttle, not a security boundary. Negligible practical impact.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability properly enforced via RolePermissions() deep copy function and RolePermissionsFor() single-role accessor.
[authentication] internal/mintcore/oidc.go — JWKSVerifier is well-constructed: algorithm pinned to RS256, kid header required, JWKS URI origin validated against issuer URL, JWKS cache with 1h TTL and 30s minimum refresh, singleflight for concurrent fetches, maxJWKSResponseLen (512KB) cap, and full claims validation including ValidateOrgAllowed and ValidateWorkflowRef.
[authorization] internal/mintcore/github.go — rolePermissions is now unexported with deep-copy accessor functions (RolePermissions, RolePermissionsFor, HasRole), preventing callers from mutating the canonical permission definitions at runtime. This is a security improvement over the prior mutable exported map.
[injection] internal/mintcore/claims.go:ValidateWorkflowRef — Now derives the .fullsend config prefix from the repository claim's owner rather than iterating allowedOrgs, which is more precise and prevents cross-org confusion. The prior medium finding about the unused allowedOrgs parameter has been resolved — the parameter was removed entirely.
[test-adequacy] internal/mint/main_test.go — Tests that previously used fake unsigned OIDC tokens (makeTestOIDCToken + fakeTokenValidator) have been replaced with tests using real cryptographic signing via newTestOIDCEnv + JWKSVerifier. This is a strict improvement. Unit tests for Audience, Claims validation, org checking, workflow ref, and buildRepoProviderID have moved to internal/mintcore/*_test.go with equivalent or better coverage. No assertion weakening detected.
[sub-agent-failure] N/A — The intent-coherence, style-conventions, and docs-currency sub-agents did not return findings due to model availability constraints (claude-sonnet-4-5@20250929 not available on the deployment). These dimensions were not evaluated in this review run. No blocking issues were identified in the dimensions that were evaluated (correctness, security).

Previous run (4)

Review

Findings

Medium

[logic-error] internal/mintcore/claims.go:ValidateWorkflowRef — The allowedOrgs []string parameter is accepted but never referenced in the function body. Both callers (JWKSVerifier.Verify and STSVerifier.prevalidate) pass v.allowedOrgs to no effect. The old inline code used claims.RepositoryOwner for the .fullsend/ config prefix (not allowedOrgs), so this parameter was dead on arrival during extraction. An unused parameter in a security-critical validation function is misleading — a future maintainer might assume org-scoping is enforced here when it is not.
Remediation: Remove the allowedOrgs parameter from ValidateWorkflowRef and update both call sites in oidc.go and sts.go.
[data-exposure] internal/mint/main.go:handleStatus — The /v1/status endpoint now returns both the full allowedOrgs list and org-stripped role names to any caller with a valid OIDC token from any allowed org. The prior review noted that roleAppIDs keys leaked org-scoped role keys; this version improves by stripping the org prefix from roles, but now explicitly returns the orgs list (which was not previously exposed). The combination still reveals the deployment matrix — which organizations use the mint and what roles are configured — aiding reconnaissance. See also: [authorization] finding on this endpoint.
Remediation: Return only roles available for the requesting org (derived from the authenticated token's repository_owner claim) or restrict the endpoint to a specific admin role.

Low

[authorization] internal/mint/main.go — The /v1/status endpoint performs OIDC verification but does not check whether the authenticated caller has any specific role or elevated privilege. Any workflow from any allowed org with a valid OIDC token (including triage-level workflows) can enumerate all configured orgs and roles. See also: [data-exposure] finding on this endpoint.
[logic-error] internal/mint/main.go:ServeHTTP — The auth header check now runs before the POST method check for /v1/token and / paths. This means GET /v1/token without auth returns 401 instead of the previous 405. Arguably more secure (avoids leaking endpoint existence to unauthenticated callers) but is an undocumented behavior change.
[missing-test] internal/mint/main_test.go — Many unit tests that previously tested individual functions in isolation (TestPrevalidateOIDCToken_*, TestAudienceUnmarshalJSON, TestCheckAllowedOrg, etc.) were removed. Most are now covered by equivalent tests in internal/mintcore/*_test.go. The new TestHandler_RoleAllowed tests via a full OIDC flow through newTestOIDCEnv, which is more realistic but makes it harder to isolate role-checking logic failures. Net test coverage appears adequate.
[edge-case] internal/mintcore/oidc.go:refreshKeys — After the first successful JWKS discovery, the cachedJWKSURI is never invalidated. Changes to the OIDC discovery document's jwks_uri (e.g., key infrastructure migration) will never be picked up — the mint process must be restarted. This is a deliberate design tradeoff (avoids two HTTP requests per refresh).
[edge-case] internal/dispatch/gcf/provisioner.go:addDirToZip — The function skips directories, so if internal/mintcore/ ever gains subdirectories, those files would be silently excluded from the Cloud Function deployment zip. Fine today since mintcore is flat.
[race-condition] internal/mintcore/oidc.go:getKey — Between refreshGroup.Do returning and the subsequent v.mu.Lock(), another goroutine could trigger a new refresh cycle. The consequence is a slightly stale lastKidMissAt timestamp — a performance throttle, not a security boundary. Negligible practical impact.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability properly enforced via RolePermissions() deep copy function and RolePermissionsFor() single-role accessor.
[authentication] internal/mintcore/oidc.go — JWKSVerifier is well-constructed: algorithm pinned to RS256, kid header required, JWKS URI origin validated against issuer URL, JWKS cache with 1h TTL and 30s minimum refresh, singleflight for concurrent fetches, maxJWKSResponseLen (512KB) cap, and full claims validation including ValidateOrgAllowed and ValidateWorkflowRef.
[authorization] internal/mintcore/github.go — rolePermissions is now unexported with deep-copy accessor functions (RolePermissions, RolePermissionsFor), preventing callers from mutating the canonical permission definitions at runtime. This is a security improvement over the prior mutable exported map.
[injection] internal/mintcore/claims.go:ValidateWorkflowRef — Now derives the .fullsend config prefix from the repository claim's owner rather than repository_owner, which is more precise and prevents cross-org confusion.
[sub-agent-failure] N/A — The intent-coherence, style-conventions, docs-currency, and cross-repo-contracts sub-agents did not return findings due to model availability constraints (claude-sonnet-4-5@20250929 not available on the deployment). These dimensions were not evaluated in this review run. No blocking issues were identified in the dimensions that were evaluated (correctness, security).

Previous run (5)

Review

Findings

Medium

[data-exposure] internal/mint/main.go:handleStatus — The /v1/status endpoint returns roleAppIDs keys (e.g., org-a/coder, org-b/review), which reveals the full org-role deployment matrix to any caller with a valid OIDC token from any allowed org. While the endpoint correctly requires authentication and does not leak app IDs or secrets, the org-scoped role keys expose which organizations use the mint and what roles they have configured. This information could aid reconnaissance for targeted privilege escalation attempts.
Remediation: Return only role names (strip the org prefix) or restrict the endpoint to a specific admin role rather than accepting any valid OIDC token.

Low

[logic-error] internal/mint/main.go:ServeHTTP — The auth header check now runs before the POST method check for /v1/token and / paths (previously, method check came first). This means GET /v1/token without auth returns 401 instead of the previous 405. This is arguably more secure (avoids leaking endpoint existence to unauthenticated callers) but is an undocumented behavior change.
[naming-convention] internal/mintcore/github.go:RolePermissions — RolePermissions() returns all roles while RolePermissionsFor(role) returns one. Consider AllRolePermissions() to make the distinction clearer, though the current naming follows Go conventions (cf. http.Request.Cookies() vs Cookie(name)).
[missing-test] internal/mint/main_test.go — TestHandler_RoleAllowed was deleted rather than migrated. The role-acceptance path is now covered implicitly by full-flow tests (TestHandler_FullFlow, TestHandler_STSVerifier_Integration), but there is no isolated test verifying that a valid role passes validation while an unknown role is rejected.
[edge-case] internal/mintcore/oidc.go:discoverJWKSURI — The OIDC discovery document is fetched on every JWKS cache refresh (two HTTP requests per refresh). Consider caching the jwks_uri value for the same TTL as the JWKS keys to reduce the failure surface if the discovery endpoint is slow.
[incomplete-doc] CLAUDE.md — The new mintcore sync instructions are accurate but could note that provisioner.go rewrites the go.mod replace directive from => ../mintcore to => ./mintcore at bundle time. Without this context, a maintainer syncing .embed files might be confused by the different paths.

Previous run (6)

Review

Findings

Low

[race-condition] internal/mintcore/oidc.go:getKey — When the JWKS cache has expired but the key IS found, getKey attempts a refresh. If refreshKeys fails (network error, bad response), the error is returned and the previously-cached valid key is not used. This is a fail-closed design choice (safer than serving with potentially rotated keys) and only affects the JWKSVerifier path (not the production STSVerifier). However, for the upcoming devmint, a transient JWKS endpoint failure could reject all tokens even when the stale cache would have been valid.
Remediation: Consider falling back to the stale cached key when refresh fails and the key was previously found: if ok && err != nil { return key, nil }.
[test-inadequate] internal/mint/main_test.go — TestHandler_STSVerifier_Integration exercises the STSVerifier through the full ServeHTTP path with a mock STS server. TestHandler_STSVerifier_RestrictedWorkflows validates workflow restrictions through the STSVerifier path. The remaining gap is narrow: no test exercises per-repo WIF workflow restrictions specifically through the STSVerifier path via ServeHTTP (the per-repo test TestHandler_PerRepoWIF_RestrictedWorkflows uses JWKSVerifier), but ValidateWorkflowRef is shared code called identically by both verifiers.
[data-exposure] internal/mint/main.go — The /v1/status endpoint returns the full list of allowed orgs and role-to-AppID mapping keys to any caller with a valid OIDC token. Authentication is required and the data is not secret. TestHandler_StatusEndpoint explicitly verifies no app IDs are leaked (only keys exposed). This is by design for the status endpoint's diagnostic purpose.

Info

[error-handling] internal/mint/main.go:ServeHTTP — The /v1/status path returns 401 Unauthorized on OIDC verification failure, while the /v1/token path returns 403 Forbidden for the same failure. The status endpoint is semantically more correct (401 = invalid credentials); the token endpoint's 403 is pre-existing behavior. Minor inconsistency, no functional impact.
[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability properly enforced via RolePermissions() deep copy function and RolePermissionsFor() single-role accessor.
[authorization] internal/mintcore/github.go — rolePermissions is now unexported with deep-copy accessor functions (RolePermissions, RolePermissionsFor), preventing callers from mutating the canonical permission definitions at runtime. This is a security improvement over the prior mutable exported map.
[authentication] internal/mintcore/oidc.go — JWKSVerifier is well-constructed: algorithm pinned to RS256, kid header required, JWKS URI origin validated against issuer URL, JWKS cache with 1h TTL and 30s minimum refresh, singleflight for concurrent fetches, maxJWKSResponseLen (512KB) cap, and full claims validation. ValidateWorkflowRef now derives the .fullsend config prefix from the repository claim's owner rather than repository_owner, which is stricter and more correct.
[secrets-handling] internal/mint/main.go — Error responses from Secret Manager continue to be drained and discarded (io.Copy to io.Discard with 4KB limit) to prevent leaking resource paths. Safe behavior preserved.
[prior-resolved] Prior low [test-inadequate] on handler-level STSVerifier integration tests — confirmed addressed. TestHandler_STSVerifier_RestrictedWorkflows was added covering workflow restrictions through the STS path. Remaining gap narrowed to per-repo WIF via STS only.
[sub-agent-failure] N/A — The intent-coherence, style-conventions, and docs-currency sub-agents did not return findings due to model availability constraints (claude-sonnet-4-5@20250929 not available on the deployment). These dimensions were not evaluated in this review run. No blocking issues were identified in the dimensions that were evaluated (correctness, security).

Previous run (7)

Review

Findings

Low

[test-inadequate] internal/mint/main_test.go — The new TestHandler_STSVerifier_Integration exercises the STSVerifier through the full ServeHTTP path with a mock STS server, addressing the prior medium-severity gap. Workflow file restriction is tested through ServeHTTP via TestHandler_RestrictedWorkflowFiles (JWKSVerifier) and TestHandler_PerRepoWIF_RestrictedWorkflows (JWKSVerifier). The remaining gap is narrow: no test exercises restricted workflows specifically through the STSVerifier path via ServeHTTP, but ValidateWorkflowRef is shared code called identically by both verifiers, so coverage through either path validates the logic.
Remediation: Consider adding a TestHandler_STSVerifier_RestrictedWorkflows test for completeness.
[data-exposure] internal/mint/main.go — The /v1/status endpoint returns the full list of allowed orgs and role-to-AppID mapping keys to any caller with a valid OIDC token. Authentication is required and the data is not secret. The TestHandler_StatusEndpoint test explicitly verifies no app IDs are leaked (only keys exposed). This is by design for the status endpoint's diagnostic purpose.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability is properly enforced via RolePermissions() deep copy function and RolePermissionsFor() single-role accessor.
[design-direction] internal/mintcore/claims.go:ValidateWorkflowRef — Behavioral improvement: the new code derives the .fullsend config prefix from the repository claim's owner (not repository_owner), which is more precise. A token with mismatched repository and repository_owner claims now has the .fullsend prefix checked against the correct org.
[prior-resolved] Prior medium [test-inadequate] on handler-level STSVerifier integration tests — improved in this revision. TestHandler_STSVerifier_Integration now exercises the STSVerifier through ServeHTTP with a mock STS server, confirming the verifier-to-handler wiring is correct. TestHandler_RestrictedWorkflowFiles and TestHandler_PerRepoWIF_RestrictedWorkflows exercise workflow restriction through ServeHTTP. Coverage gap narrowed from medium to low.
[authentication] internal/mintcore/oidc.go — JWKSVerifier is well-constructed: algorithm pinned to RS256, kid header required, JWKS URI origin validated against issuer URL, JWKS cache with 1h TTL and 30s minimum refresh, singleflight for concurrent fetches, and full claims validation (issuer, audience, expiry, iat, repository, org, workflow ref).
[sub-agent-failure] N/A — The intent-coherence, style-conventions, and docs-currency sub-agents did not return findings due to model availability constraints on the deployment. These dimensions were not evaluated in this review run. No blocking issues were identified in the dimensions that were evaluated (correctness, security).

Previous run (8)

Review

Findings

Medium

[test-inadequate] internal/mint/main_test.go — Handler-level integration tests for the production STSVerifier path were removed. The PR adds a dedicated TestHandler_RestrictedWorkflowFiles test that exercises workflow-file restriction through the full ServeHTTP path using JWKSVerifier, which partially addresses the gap. However, several prior handler-level tests (per-repo WIF cross-repo, per-repo non-workflow path, upstream non-workflow path) are now only tested at the ValidateWorkflowRef unit level in claims_test.go, not through ServeHTTP. The shared ValidateWorkflowRef function is well-tested in isolation, and the new integration test confirms the verifier-to-handler wiring works, but the STSVerifier-specific production wiring path lacks end-to-end coverage.
Remediation: Consider adding at least one ServeHTTP-level integration test that exercises a per-repo WIF workflow restriction (not wildcard) to verify the verifier plumbing is correct end-to-end for that code path.

Low

[data-exposure] internal/mint/main.go — The /v1/status endpoint returns the full list of allowed orgs and all role-to-AppID mapping keys to any caller with a valid OIDC token. While authentication is required and the data is not secret, it exposes the complete RBAC topology (enrolled organizations, configured roles per org) in a single response, aiding reconnaissance for an attacker who has compromised any workflow in an allowed org. This is by design for the status endpoint's diagnostic purpose.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability is properly enforced via deep copies.
[design-direction] internal/mintcore/claims.go:ValidateWorkflowRef — Behavioral improvement: the new code derives the .fullsend config prefix from the repository claim's owner (not repository_owner), which is more precise. A token with mismatched repository and repository_owner claims now has the .fullsend prefix checked against the correct org.
[prior-resolved] Prior medium [auth-bypass] on JWKSVerifier.Verify skipping audience check when empty — resolved in this revision. Both JWKSVerifier.Verify and STSVerifier.prevalidate now fail closed with "OIDC audience must be configured" when audience is empty. Prior low [pattern-violation] on addDirToZip symlinks — resolved; the function now explicitly checks entry.Type()&os.ModeSymlink.

Previous run (9)

Review

Findings

Medium

[auth-bypass] internal/mintcore/oidc.go — JWKSVerifier.Verify skips the audience check when v.audience is empty (if v.audience != "" && ...), silently accepting tokens with any audience. By contrast, STSVerifier.prevalidate fails closed with "OIDC audience must be configured" when audience is empty. This inconsistency means a JWKSVerifier constructed without an audience (e.g., a misconfigured deployment) will silently accept tokens minted for any audience, defeating audience binding. JWKSVerifier is not in the production path today (GCP mint uses STSVerifier), but it will be used by the devmint in the follow-up PR and the comment says "for future dev mint and non-GCP deployments."
Remediation: Align with STSVerifier — return an error when v.audience is empty instead of skipping the check. If the empty-audience behavior is intentional for --insecure-no-auth mode, document that explicitly and consider a separate constructor or option flag.
[test-inadequate] internal/mint/main_test.go — Several workflow file restriction integration tests were removed (TestHandler_OIDCPrevalidation_WorkflowAllowlist, TestHandler_OIDCPrevalidation_WorkflowAllowlistUnset, per-repo and upstream variants). The new newTestOIDCEnv helper constructs a JWKSVerifier with AllowedWorkflowFiles: []string{"*"} (wildcard), so no integration test verifies that the handler rejects tokens with disallowed workflow files through the full ServeHTTP path. The mintcore unit tests cover ValidateWorkflowRef directly, but the integration path is untested.
Remediation: Add at least one integration test that creates a newTestOIDCEnv with a restricted AllowedWorkflowFiles list and verifies rejection.

Low

[coverage-reduced] internal/mintcore/claims_test.go — Whitespace-only audience test cases (" ", [" "]) from the original TestAudienceUnmarshalJSON were not carried over. TestValidateOrgAllowed has 4 cases vs. the original TestCheckAllowedOrg with 10 cases covering edge cases like empty env and whitespace-trimmed entries.
[data-exposure] internal/mint/main.go:handleStatus — The /v1/status endpoint returns the full list of allowed orgs and all role-to-AppID mappings to any caller with a valid OIDC token. While the endpoint requires authentication and the data is largely public, it reveals the complete set of enrolled organizations and installed GitHub App IDs in a single response, aiding reconnaissance.
[pattern-violation] internal/dispatch/gcf/provisioner.go:addDirToZip — The new addDirToZip function does not check for symlinks before reading files (entry.Type()&os.ModeSymlink), unlike the parent bundleFunctionSource which explicitly skips symlinks at line 1703. A symlinked file in the mintcore directory could cause unintended files to be included in the deployment zip.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability is properly enforced via deep copies.
[design-direction] internal/mintcore/claims.go:ValidateWorkflowRef — Behavioral improvement: the new code derives the .fullsend config prefix from the repository claim's owner (not repository_owner), which is more precise. In the original code, a token with mismatched repository and repository_owner claims could have the .fullsend prefix checked against the wrong org.

Previous run (10)

Review

Findings

Medium

[information-disclosure] internal/mint/main.go (new /v1/status handler) — The /v1/status endpoint returns OIDC verification error details in the HTTP response via fmt.Sprintf("OIDC verification failed: %v", err). This leaks internal validation details (issuer URLs, audience values, claim names, workflow ref paths) to callers. In contrast, the /v1/token endpoint correctly returns a generic "authentication failed" message and logs the error internally.
Remediation: Use the same generic error response as /v1/token: writeError(w, http.StatusUnauthorized, "authentication failed").
[test-coverage-reduction] internal/mint/main_test.go — Handler-level tests now use newTestOIDCEnv which injects a JWKSVerifier instead of testing through the STSVerifier path that production uses. Multiple handler-level integration tests were removed (TestHandler_OIDCPrevalidation_BadIssuer, _ExpiredToken, _WrongOrg, _BadAudience, _MissingJobWorkflowRef, _UpstreamWorkflowRef, per-repo WIF provider tests, workflow allowlist tests). While the underlying logic is unit-tested in mintcore/sts_test.go and claims_test.go, the handler+STSVerifier wiring is no longer integration-tested. See also: [missing-validation] finding on JWKSVerifier.
Remediation: Add at least one handler-level integration test that constructs a Handler with an STSVerifier (pointing at a mock STS server) to verify the production OIDC flow end-to-end.
[scope-exceeded] internal/mint/main.go, internal/mintcore/oidc.go — The PR is titled "refactor: extract mintcore shared library" and described as a "pure refactoring," but it includes new features: (1) a /v1/status endpoint with new types (statusResponse, statusRole) and OIDC-authenticated handler, and (2) JWKSVerifier (~150 lines of new JWKS-based OIDC verification code) that is not used in the production Cloud Function path. Both are authorized by issue feat: add mintcore shared library and dev mint for local evaluation #1751, but the PR title/description should reflect the actual scope to aid reviewers.
Remediation: Update PR title to feat: (or refactor!:) and explicitly note the new features in the description.

Low

[privilege-escalation] internal/mintcore/github.go:47 — RolePermissions is now an exported package-level var (mutable map). Previously it was unexported (rolePermissions) in the mint package. Any code importing mintcore can mutate the map at runtime (e.g., mintcore.RolePermissions["triage"]["contents"] = "write"), silently escalating permissions for all subsequently minted tokens. Mitigated by internal/ visibility, but the exposure surface increased from 1 to N consumers.
[information-disclosure] internal/mint/main.go (new /v1/status handler) — The /v1/status endpoint exposes the full list of allowed orgs and role-to-appID mappings to any caller with a valid OIDC token. GitHub App IDs are not secret, and the endpoint requires authentication, but the data aids reconnaissance.
[missing-validation] internal/mintcore/oidc.go — JWKSVerifier.Verify does not call ValidateOrgAllowed or ValidateWorkflowRef after parsing claims. Unlike STSVerifier.prevalidate, which checks both, JWKSVerifier accepts tokens from any org/workflow as long as signature, issuer, audience, and timestamps are valid. This weakens defense-in-depth for the upcoming devmint and reduces test fidelity when JWKSVerifier is used as the handler's OIDCVerifier in tests.
[security-comment-removal] internal/mint/main.go:91 — Security-rationale comments were removed from smPEMAccessor.AccessPEM explaining why response bodies are discarded (to avoid leaking secret resource names). The safe behavior (io.Copy to io.Discard) is preserved but the reasoning is lost for future maintainers.
[logic-change] internal/mintcore/claims.go:73 — ValidateWorkflowRef now iterates over all allowedOrgs to check .fullsend config prefixes, whereas the old code only checked claims.RepositoryOwner. Functionally equivalent for legitimate tokens (org is pre-validated), but subtly broader: a token from org-A could match org-B's .fullsend/ prefix if both are in the allowed list.
[deployment-risk] internal/dispatch/gcf/provisioner.go:1756 — addDirToZip silently returns nil when srcDir does not exist. If the mintcore directory is missing at bundle time, the zip will be created without it. Mitigated by the fact that bundleEmbeddedMintSource (the production deployment path) uses embedded files, not disk reads.
[missing-embed] internal/dispatch/gcf/provisioner.go — mintcore/go.sum is not included in the embedded files. The test explicitly skips go.sum. Since mintcore's only direct dependency (testify) is test-only and no non-test files import it, go build should succeed. But if a non-test dependency is added, the embedded deployment path will break.
[stale-doc] docs/guides/infrastructure/infrastructure-reference.md:66 — References rolePermissions (lowercase unexported name), but the PR exports this as mintcore.RolePermissions. The diagram text should be updated to match.
[pattern-inconsistency] internal/mintcore/*_test.go — New mintcore tests use github.com/stretchr/testify (assert/require), while the existing internal/mint/main_test.go uses standard testing.T with t.Fatalf. Inconsistent testing patterns across closely related internal packages.

Previous run (11)

Review

Findings

Medium

[test-inadequate] internal/mint/main_test.go — Handler-level integration tests for the production STSVerifier path were removed. The PR adds a dedicated TestHandler_RestrictedWorkflowFiles test that exercises workflow-file restriction through the full ServeHTTP path using JWKSVerifier, which partially addresses the gap. However, several prior handler-level tests (per-repo WIF cross-repo, per-repo non-workflow path, upstream non-workflow path) are now only tested at the ValidateWorkflowRef unit level in claims_test.go, not through ServeHTTP. The shared ValidateWorkflowRef function is well-tested in isolation, and the new integration test confirms the verifier-to-handler wiring works, but the STSVerifier-specific production wiring path lacks end-to-end coverage.
Remediation: Consider adding at least one ServeHTTP-level integration test that exercises a per-repo WIF workflow restriction (not wildcard) to verify the verifier plumbing is correct end-to-end for that code path.

Low

[data-exposure] internal/mint/main.go — The /v1/status endpoint returns the full list of allowed orgs and all role-to-AppID mapping keys to any caller with a valid OIDC token. While authentication is required and the data is not secret, it exposes the complete RBAC topology (enrolled organizations, configured roles per org) in a single response, aiding reconnaissance for an attacker who has compromised any workflow in an allowed org. This is by design for the status endpoint's diagnostic purpose.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability is properly enforced via deep copies.
[design-direction] internal/mintcore/claims.go:ValidateWorkflowRef — Behavioral improvement: the new code derives the .fullsend config prefix from the repository claim's owner (not repository_owner), which is more precise. A token with mismatched repository and repository_owner claims now has the .fullsend prefix checked against the correct org.
[prior-resolved] Prior medium [auth-bypass] on JWKSVerifier.Verify skipping audience check when empty — resolved in this revision. Both JWKSVerifier.Verify and STSVerifier.prevalidate now fail closed with "OIDC audience must be configured" when audience is empty. Prior low [pattern-violation] on addDirToZip symlinks — resolved; the function now explicitly checks entry.Type()&os.ModeSymlink.

Previous run (12)

Review

Findings

Medium

[auth-bypass] internal/mintcore/oidc.go — JWKSVerifier.Verify skips the audience check when v.audience is empty (if v.audience != "" && ...), silently accepting tokens with any audience. By contrast, STSVerifier.prevalidate fails closed with "OIDC audience must be configured" when audience is empty. This inconsistency means a JWKSVerifier constructed without an audience (e.g., a misconfigured deployment) will silently accept tokens minted for any audience, defeating audience binding. JWKSVerifier is not in the production path today (GCP mint uses STSVerifier), but it will be used by the devmint in the follow-up PR and the comment says "for future dev mint and non-GCP deployments."
Remediation: Align with STSVerifier — return an error when v.audience is empty instead of skipping the check. If the empty-audience behavior is intentional for --insecure-no-auth mode, document that explicitly and consider a separate constructor or option flag.
[test-inadequate] internal/mint/main_test.go — Several workflow file restriction integration tests were removed (TestHandler_OIDCPrevalidation_WorkflowAllowlist, TestHandler_OIDCPrevalidation_WorkflowAllowlistUnset, per-repo and upstream variants). The new newTestOIDCEnv helper constructs a JWKSVerifier with AllowedWorkflowFiles: []string{"*"} (wildcard), so no integration test verifies that the handler rejects tokens with disallowed workflow files through the full ServeHTTP path. The mintcore unit tests cover ValidateWorkflowRef directly, but the integration path is untested.
Remediation: Add at least one integration test that creates a newTestOIDCEnv with a restricted AllowedWorkflowFiles list and verifies rejection.

Low

[coverage-reduced] internal/mintcore/claims_test.go — Whitespace-only audience test cases (" ", [" "]) from the original TestAudienceUnmarshalJSON were not carried over. TestValidateOrgAllowed has 4 cases vs. the original TestCheckAllowedOrg with 10 cases covering edge cases like empty env and whitespace-trimmed entries.
[data-exposure] internal/mint/main.go:handleStatus — The /v1/status endpoint returns the full list of allowed orgs and all role-to-AppID mappings to any caller with a valid OIDC token. While the endpoint requires authentication and the data is largely public, it reveals the complete set of enrolled organizations and installed GitHub App IDs in a single response, aiding reconnaissance.
[pattern-violation] internal/dispatch/gcf/provisioner.go:addDirToZip — The new addDirToZip function does not check for symlinks before reading files (entry.Type()&os.ModeSymlink), unlike the parent bundleFunctionSource which explicitly skips symlinks at line 1703. A symlinked file in the mintcore directory could cause unintended files to be included in the deployment zip.

Info

[extraction-faithfulness] internal/mintcore/sts.go — The STSVerifier extraction is faithful: all validation checks (issuer, audience, clock skew, repository, org allowlist, workflow ref, STS exchange, WIF provider resolution) are preserved with identical logic. The cross-org installation mismatch guard in FindInstallation is intact. RolePermissions immutability is properly enforced via deep copies.
[design-direction] internal/mintcore/claims.go:ValidateWorkflowRef — Behavioral improvement: the new code derives the .fullsend config prefix from the repository claim's owner (not repository_owner), which is more precise. In the original code, a token with mismatched repository and repository_owner claims could have the .fullsend prefix checked against the wrong org.

Previous run (13)

Review

Findings

Medium

[information-disclosure] internal/mint/main.go (new /v1/status handler) — The /v1/status endpoint returns OIDC verification error details in the HTTP response via fmt.Sprintf("OIDC verification failed: %v", err). This leaks internal validation details (issuer URLs, audience values, claim names, workflow ref paths) to callers. In contrast, the /v1/token endpoint correctly returns a generic "authentication failed" message and logs the error internally.
Remediation: Use the same generic error response as /v1/token: writeError(w, http.StatusUnauthorized, "authentication failed").
[test-coverage-reduction] internal/mint/main_test.go — Handler-level tests now use newTestOIDCEnv which injects a JWKSVerifier instead of testing through the STSVerifier path that production uses. Multiple handler-level integration tests were removed (TestHandler_OIDCPrevalidation_BadIssuer, _ExpiredToken, _WrongOrg, _BadAudience, _MissingJobWorkflowRef, _UpstreamWorkflowRef, per-repo WIF provider tests, workflow allowlist tests). While the underlying logic is unit-tested in mintcore/sts_test.go and claims_test.go, the handler+STSVerifier wiring is no longer integration-tested. See also: [missing-validation] finding on JWKSVerifier.
Remediation: Add at least one handler-level integration test that constructs a Handler with an STSVerifier (pointing at a mock STS server) to verify the production OIDC flow end-to-end.
[scope-exceeded] internal/mint/main.go, internal/mintcore/oidc.go — The PR is titled "refactor: extract mintcore shared library" and described as a "pure refactoring," but it includes new features: (1) a /v1/status endpoint with new types (statusResponse, statusRole) and OIDC-authenticated handler, and (2) JWKSVerifier (~150 lines of new JWKS-based OIDC verification code) that is not used in the production Cloud Function path. Both are authorized by issue feat: add mintcore shared library and dev mint for local evaluation #1751, but the PR title/description should reflect the actual scope to aid reviewers.
Remediation: Update PR title to feat: (or refactor!:) and explicitly note the new features in the description.

Low

[privilege-escalation] internal/mintcore/github.go:47 — RolePermissions is now an exported package-level var (mutable map). Previously it was unexported (rolePermissions) in the mint package. Any code importing mintcore can mutate the map at runtime (e.g., mintcore.RolePermissions["triage"]["contents"] = "write"), silently escalating permissions for all subsequently minted tokens. Mitigated by internal/ visibility, but the exposure surface increased from 1 to N consumers.
[information-disclosure] internal/mint/main.go (new /v1/status handler) — The /v1/status endpoint exposes the full list of allowed orgs and role-to-appID mappings to any caller with a valid OIDC token. GitHub App IDs are not secret, and the endpoint requires authentication, but the data aids reconnaissance.
[missing-validation] internal/mintcore/oidc.go — JWKSVerifier.Verify does not call ValidateOrgAllowed or ValidateWorkflowRef after parsing claims. Unlike STSVerifier.prevalidate, which checks both, JWKSVerifier accepts tokens from any org/workflow as long as signature, issuer, audience, and timestamps are valid. This weakens defense-in-depth for the upcoming devmint and reduces test fidelity when JWKSVerifier is used as the handler's OIDCVerifier in tests.
[security-comment-removal] internal/mint/main.go:91 — Security-rationale comments were removed from smPEMAccessor.AccessPEM explaining why response bodies are discarded (to avoid leaking secret resource names). The safe behavior (io.Copy to io.Discard) is preserved but the reasoning is lost for future maintainers.
[logic-change] internal/mintcore/claims.go:73 — ValidateWorkflowRef now iterates over all allowedOrgs to check .fullsend config prefixes, whereas the old code only checked claims.RepositoryOwner. Functionally equivalent for legitimate tokens (org is pre-validated), but subtly broader: a token from org-A could match org-B's .fullsend/ prefix if both are in the allowed list.
[deployment-risk] internal/dispatch/gcf/provisioner.go:1756 — addDirToZip silently returns nil when srcDir does not exist. If the mintcore directory is missing at bundle time, the zip will be created without it. Mitigated by the fact that bundleEmbeddedMintSource (the production deployment path) uses embedded files, not disk reads.
[missing-embed] internal/dispatch/gcf/provisioner.go — mintcore/go.sum is not included in the embedded files. The test explicitly skips go.sum. Since mintcore's only direct dependency (testify) is test-only and no non-test files import it, go build should succeed. But if a non-test dependency is added, the embedded deployment path will break.
[stale-doc] docs/guides/infrastructure/infrastructure-reference.md:66 — References rolePermissions (lowercase unexported name), but the PR exports this as mintcore.RolePermissions. The diagram text should be updated to match.
[pattern-inconsistency] internal/mintcore/*_test.go — New mintcore tests use github.com/stretchr/testify (assert/require), while the existing internal/mint/main_test.go uses standard testing.T with t.Fatalf. Inconsistent testing patterns across closely related internal packages.

waynesun09

Review Squad Report — 7 agents, 11 findings (MEDIUM+)

Agents: 2x claude-coder, 2x claude-researcher, 2x gemini-code-review, 1x cursor-code-review
Total findings: 21 (after dedup and false positive removal) — 3 HIGH, 8 MEDIUM, 7 LOW, 3 INFO
False positives removed: 5

Summary

The STSVerifier extraction is faithful — identical validation logic, error messages, and control flow preserved. The most actionable finding is the JWKSVerifier fail-open design on empty allowlists (3-agent consensus), which creates a security asymmetry with STSVerifier that should be fixed before devmint uses it. The status endpoint App ID exposure (4-agent consensus) and duplicate BuildRepoProviderID are cleanup items to address in this PR. Four deleted test suites covering security-relevant edge cases should be restored in mintcore.

Only MEDIUM+ findings are posted inline. 7 LOW and 3 INFO findings available on request.

ggallen · 2026-06-02T15:58:53Z

All findings from both review sources have been addressed in fc4843d2:

From review #4411014072 (8 inline comments): All fixed and threads resolved.

From issue comment #4603468794:

[auth-bypass] MEDIUM — Fixed: JWKSVerifier now errors on empty audience (fail-closed, matching STSVerifier)
[test-inadequate] MEDIUM — Fixed: Added TestHandler_RestrictedWorkflowFiles integration test
[coverage-reduced] LOW — Fixed: Added whitespace audience edge case tests and TestValidateOrgAllowed_EmptyList
[data-exposure] LOW — Fixed: Removed App IDs from /v1/status response
[pattern-violation] LOW — Fixed: Added symlink check in addDirToZip

Extract shared types, validation, and GitHub API helpers from the monolithic mint Cloud Function into a reusable mintcore module. The GCP mint now imports mintcore for claims validation, JWT generation, and installation token creation. Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fullsend-ai-review

See the review comment for full details.

waynesun09

All 11 HIGH/MEDIUM findings from the prior review round are resolved. The update is thorough — JWKSVerifier is now fail-closed, status endpoint is org-scoped with no App ID exposure, singleflight coalesces JWKS refreshes, routing is correct, and all test gaps were restored.

Two minor items for follow-up (non-blocking):

STSVerifierOptions.AllowedWorkflows → AllowedWorkflowFiles for consistency with JWKSVerifierOptions
Add TestSTSVerifier_MissingIat, TestSTSVerifier_FutureToken, TestSTSVerifier_MissingRepository to match JWKS verifier test coverage

…on tests Address review findings from PR fullsend-ai#1783: Medium: - Add issuer mismatch check in discoverJWKSURI per OIDC Discovery spec Section 4.3: verify doc.Issuer == expected issuerURL before trusting the discovery document. Applied to both mintcore and .embed. - Add 5 handler-level integration tests restoring end-to-end coverage for scenarios removed during mintcore extraction: - Upstream workflow ref acceptance through ServeHTTP - Per-repo cross-repo ref rejection - Non-workflow path rejection - Unregistered per-repo WIF repo rejection - Per-repo mixed-case matching Low findings verified as already addressed: - getKey already uses RLock on the hot path (line 195) - addDirToZip already has symlink detection (line 1756) - /v1/status data-exposure is documented as intentional Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

ggallen · 2026-06-03T00:08:20Z

Response to review findings (head SHA `51a084cb`)

All three low findings are no-ops:

Finding #1 (data-exposure, /v1/status role enumeration): Accepted by design. The endpoint is gated behind OIDC verification and returns only the caller's own org roles — not app IDs, not cross-org data. This has been flagged and accepted across 8+ prior review rounds. Role names (triage, code, review, fix) are not sensitive; they're documented publicly in the infrastructure reference guide.

Finding #2 (error-handling, addDirToZip subdirectory symlinks): False positive. os.ReadDir uses Lstat semantics — a symlinked directory has Type() == os.ModeSymlink with IsDir() == false. The symlink check at line 1756 runs before the IsDir() check at line 1759, so symlinked directories are skipped. Verified empirically:

name=linkdir    type=L---------     isDir=false isSymlink=true
name=realdir    type=d---------     isDir=true  isSymlink=false

Finding #3 (test-coverage-reduction, edge case verification): Covered. The mintcore package has 31 unit tests covering malformed tokens, expired claims, wrong issuer/audience/org/workflow, missing fields, empty audience fail-closed, key rotation, discovery issuer mismatch, and JWKS URI origin validation. The handler tests (40 tests) include full-flow integration through both JWKSVerifier and STSVerifier paths, plus dedicated per-repo WIF, workflow restriction, cross-repo ref rejection, and case-insensitive matching tests.

ggallen mentioned this pull request Jun 2, 2026

feat: add development-only local mint server #1784

Open

4 tasks

github-actions Bot deployed to site-preview June 2, 2026 14:22 View deployment

ggallen mentioned this pull request Jun 2, 2026

feat: add mintcore shared library and dev mint for local evaluation #1751

Open

6 tasks

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

ggallen changed the title ~~refactor: extract mintcore shared library from GCP mint~~ feat: extract mintcore shared library with JWKSVerifier and status endpoint Jun 2, 2026

ggallen force-pushed the refactor/mintcore branch from 9806077 to e5a32f6 Compare June 2, 2026 14:50

github-actions Bot deployed to site-preview June 2, 2026 14:52 View deployment

ralphbean mentioned this pull request Jun 2, 2026

Review agent: post inline comments for comment-only verdicts #1787

Open

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

waynesun09 reviewed Jun 2, 2026

View reviewed changes

ggallen force-pushed the refactor/mintcore branch from e5a32f6 to fc4843d Compare June 2, 2026 15:54

github-actions Bot deployed to site-preview June 2, 2026 15:56 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 refactor/mintcore branch from fc4843d to 60e46b1 Compare June 2, 2026 16:19

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

ggallen force-pushed the refactor/mintcore branch from 60e46b1 to 9f4015b Compare June 2, 2026 16:22

github-actions Bot deployed to site-preview June 2, 2026 16:26 View deployment

fullsend-ai-review Bot approved these changes Jun 2, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Jun 2, 2026

ggallen force-pushed the refactor/mintcore branch from 9f4015b to a2ed9bb Compare June 2, 2026 17:34

github-actions Bot deployed to site-preview June 2, 2026 17:36 View deployment

fullsend-ai-review Bot approved these changes Jun 2, 2026

View reviewed changes

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jun 2, 2026

ggallen force-pushed the refactor/mintcore branch from a2ed9bb to 6d173b4 Compare June 2, 2026 18:38

github-actions Bot deployed to site-preview June 2, 2026 18:40 View deployment

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels Jun 2, 2026

ggallen force-pushed the refactor/mintcore branch from 6d173b4 to 2e7eb18 Compare June 2, 2026 18:55

github-actions Bot deployed to site-preview June 2, 2026 18:56 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 refactor/mintcore branch from 2e7eb18 to 5630f86 Compare June 2, 2026 19:13

github-actions Bot deployed to site-preview June 2, 2026 19:15 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 refactor/mintcore branch from 5630f86 to 808b21e Compare June 2, 2026 20:30

github-actions Bot deployed to site-preview June 2, 2026 20: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 2, 2026

ggallen force-pushed the refactor/mintcore branch from 808b21e to 4c2e750 Compare June 2, 2026 20:44

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

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

View reviewed changes

Comment thread internal/mintcore/oidc.go

Comment thread internal/mint/main.go

Comment thread internal/mintcore/oidc.go

Comment thread internal/dispatch/gcf/provisioner.go

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

waynesun09 approved these changes Jun 2, 2026

View reviewed changes

github-actions Bot deployed to site-preview June 2, 2026 22:20 View deployment

fullsend-ai-review Bot approved these changes Jun 2, 2026

View reviewed changes

fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 2, 2026

Conversation

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

Uh oh!

Summary

New features

What moved to mintcore

What changed in the GCP mint

Provisioner / Cloud Function deployment

Review focus

Test plan

Uh oh!

github-actions Bot commented Jun 2, 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 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review — PR #1783

Resolved prior findings

Findings

Notes

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Review

Findings

Low

Info

Review

Findings

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

ggallen commented Jun 2, 2026 •

edited

Loading

github-actions Bot commented Jun 2, 2026 •

edited

Loading

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

edited

Loading

Response to review findings (head SHA `51a084cb`)