Go Code
Mint function: The mint Cloud Function source lives in two places that must stay in sync:
internal/mint/— the source of truth (has its owngo.mod, tests run frominternal/mint/)internal/dispatch/gcf/mintsrc/— the embedded copies (.embedsuffix) deployed as a GCP Cloud Function
When changing any non-test .go file in internal/mint/, copy it to the corresponding .embed file in internal/dispatch/gcf/mintsrc/. If go.mod or go.sum changed, sync those to go.mod.embed and go.sum.embed too. The lint-mint-embed-sync pre-commit hook checks all files — not just main.go.
Standalone mint: cmd/mint/ is a standalone HTTP server variant of the token mint that serves the same purpose as the GCF mint (internal/mint/) but runs without GCP infrastructure. Both use the shared internal/mintcore/ library for token minting logic; they differ only in deployment model (filesystem PEM vs Secret Manager, JWKS vs STS verification). It supports custom role permissions via CUSTOM_ROLE_PERMISSIONS and a fallback proxy to an upstream mint. It has its own go.mod and tests run from cmd/mint/.
CF Worker adapter: internal/dispatch/cf/workersrc/ is a thin TypeScript Cloudflare Worker adapter that consumes mintcore via WASM (cmd/mint-wasm). The adapter handles I/O only (Worker secrets, host fetch, Fetch Request/Response mapping); all mint logic stays in Go. The Go WASM bridge registers mintcoreInitMint and mintcoreHandleFetch on globalThis via syscall/js; changes to these entry points in cmd/mint-wasm or to the contracts they consume in internal/mintcore/ require updating workersrc/src/index.ts to match.
Mint client: internal/mintclient/ is the Go client for calling the mint service at runtime. It exchanges a GitHub Actions OIDC JWT for a role-scoped installation token. Unlike internal/mint/ and internal/mintcore/, it has no embedded copies or sync requirements.
The internal/mintcore/ module is shared between the mint and devmint. Its files are also embedded for Cloud Function deployment at internal/dispatch/gcf/mintsrc/mintcore/*.embed. When changing any file in internal/mintcore/, sync it to the corresponding .embed file under mintsrc/mintcore/. Note: the mint's go.mod.embed uses replace mintcore => ./mintcore (not ../mintcore), because provisioner.go rewrites the replace directive at bundle time to match the deployed directory layout.
When adding a new file to internal/mintcore/:
- Create the
.embedcopy: Place it ininternal/dispatch/gcf/mintsrc/mintcore/(required for all files —lint-mint-embed-syncenforces this). - Register in
embeddedMintFiles: If the file will be included in the GCF bundle — either no build tag (e.g.,config.go) or//go:build !js(e.g.,sts_verifier.go,gcp_pem.go,wif.go) — add it toembeddedMintFilesininternal/dispatch/gcf/provisioner.goand to thego:embeddirective. - Add to
gcfSkip: If the file should NOT be in the GCF bundle — Worker-only files (//go:build js) or standalone-mint-only files — add it to thegcfSkipmap inTestEmbeddedMintSource_MatchesOriginalinprovisioner_test.goinstead ofembeddedMintFiles. The five current entries areenv_js.go,fetch_js.go,http_client_js.go, andpem_js.go(Worker-only,//go:build js) andfile_pem.go(standalone-mint-only,//go:build !js).
Verifying embed sync: After modifying any file under internal/mint/ or internal/mintcore/, run the lint script to verify all copies are in sync:
./hack/lint-mint-embed-syncYou can also run the embed test to catch desyncs:
go test -race -count=1 -run TestEmbeddedMintSource ./internal/dispatch/gcf/Both checks run in CI, but running them locally before committing catches desyncs early and avoids wasted CI iterations.
Dispatch workflows: See Workflow Contracts for dispatch sync rules, secret/input threading across installation-mode chains, and review instructions.
Interface documentation: When extending a Go interface with new methods (e.g., adding methods to ci.Driver in pkg/behaviourtest/drivers/ci/driver.go), check docs/guides/dev/ for documentation that lists or enumerates the interface's methods (e.g., behaviour-drivers.md). If found, update the method list to include all current methods, not just the newly added one. The lint-interface-doc-sync pre-commit hook enforces this for ci.Driver.
WASM binary size constraints
Files in internal/mintcore/ and cmd/mint-wasm/ are compiled to a WASM binary (GOOS=js GOARCH=wasm) for the Cloudflare Worker adapter. The compiled binary must stay within CF Workers size limits:
| Tier | Gzip limit | Makefile behavior |
|---|---|---|
| Workers Free | 3 MB | Warning |
| Workers Paid | 10 MB | Hard fail (exit 1) |
The make wasm-build target enforces these limits automatically — run it after any change to internal/mintcore/ or cmd/mint-wasm/ to verify the binary stays within bounds.
Keep the WASM dependency graph minimal. Because the Go WASM compiler includes the transitive closure of all referenced packages, small-looking changes can cause large binary size increases:
- Do not pass closures or function values (
func(string) string,func() error, etc.) into structs that are compiled into the WASM binary. Closure capture pulls the entire dependency graph of the captured variables into the binary. Prefer passing resolved values (strings, ints, config structs with only data fields) instead. - Avoid importing heavy packages in
internal/mintcore/files that are WASM-compiled. Packages likenet/http,crypto/x509, or cloud SDKs carry large dependency trees. Use build tags (//go:build js///go:build !js) to isolate platform-specific implementations. - Construct concrete verifiers at the load site (see
cmd/mint/main.go,internal/mint/main.go,cmd/mint-wasm/main.go). Each load site creates the appropriateOIDCVerifier—NewJWKSVerifierfor standalone/Worker/devmint,NewSTSVerifierfor the Cloud Function — and passes it directly intoNewHandler. Runtime constants such as the OIDC audience live inmintconsts.OIDCAudienceand are applied inside the verifier constructors, so load sites do not need to thread configuration through closures or factories. mintEnvandmintHTTPare package-internal accessors ininternal/mintcore/.NewHandlerreads configuration viamintEnv(key)and all HTTP calls go throughmintHTTP(req). On native platforms (//go:build !js),mintEnvdelegates toos.GetenvandmintHTTPuses a cached*http.Clientwith 30-second timeout. On WASM (//go:build js), the CF Worker callsRegisterEnvandRegisterHTTPonce duringmintcoreInitMintto supply JS callbacks. Do not pass HTTP clients from entrypoints into verifier configs or handler constructors — callmintHTTP(req)directly at use sites insideinternal/mintcore. Tests override the HTTP function withSetMintHTTPForTest(t, fake)and uset.Setenvfor environment variables.
When making changes to Go code under cmd/, internal/, or pkg/:
- Unit tests: Run
make go-test(orgo test ./...) and fix any failures before committing. - Coverage: CI enforces thresholds via Codecov (see
.codecov.yml). Patch coverage on changed lines must meet 80% (with a 5% tolerance). Project coverage must not drop more than 1% below the base branch.make go-testalone does not enforce these thresholds — you must verify coverage locally before committing. See Verifying patch coverage locally below for the exact commands. - Vet: Run
make go-vetto catch common issues. - E2E tests: Run
make e2e-testif your changes touchinternal/appsetup/,internal/forge/,internal/cli/, orinternal/layers/. These tests exercise the full admin install/uninstall flow against live GitHub pool orgs using mint/OIDC authentication.
Verifying patch coverage locally
make go-test runs tests with -cover but does not check whether your changed lines meet the 80% patch coverage threshold from .codecov.yml. You must approximate this check yourself before committing. Skipping this step is the most common cause of codecov/patch failures on first push.
Step-by-step
Identify changed Go files (excluding tests and generated code). Stage new files first (
git add) —git diff --name-onlyonly sees tracked or staged files, so an unstaged new file would be invisible and the check would silently skip it.bashgit diff --name-only main -- '*.go' | grep -v '_test.go'Determine affected packages from those files:
bashgit diff --name-only main -- '*.go' | grep -v '_test.go' \ | xargs -I{} dirname {} | sort -u \ | sed 's|^|./|'Run tests with a cover profile for the affected packages:
bashgo test -coverprofile=coverage.out ./path/to/changed/pkg/...If changes span multiple packages, list them all or use
./...(slower but comprehensive).Inspect per-function coverage for your changed files:
bashgo tool cover -func=coverage.out | grep 'changed_file.go'Each line shows
file:line: function coverage%. Look at functions you added or modified — these approximate Codecov's line-level patch metric.Assess against the threshold. If the functions you changed or added show coverage well below 80%, add or extend
_test.gofiles to cover the missing lines. Then re-run from step 3.
What counts as covered
Codecov measures line-level coverage on the diff. Locally, go tool cover -func reports function-level coverage, which is a coarser approximation. Target ≥ 80% on the functions you touched. If a function has complex branching, use go tool cover -html=coverage.out to visually inspect which lines are covered.
When to skip
- Test-only changes (no production
.gofiles modified) — Codecov patch coverage applies to production code, not test files. - Generated code, docs, or config-only changes — no Go coverage applies.
- Files listed in
.codecov.ymlignore:— these are excluded from coverage enforcement. Check the ignore list if your file is there.
Concurrency testing (race detection)
make go-test runs all tests with -race. Every test must pass under the race detector.
When to write a race test
When a type is shared across goroutines — for example, via World.Clone in the behaviourtest framework — write a dedicated race_test.go in the type's own package to verify thread-safety. The race detector can only catch bugs if the test exercises real concurrent access on mutable state.
Pattern: real types with forge.NewFakeClient()
Construct the real driver type backed by forge.NewFakeClient(), not a synthetic stub. Seed the FakeClient so all methods return immediately (no network, no polling). Then launch concurrent goroutines exercising representative methods and rely on -race to detect unsynchronized access.
func TestConcurrentAccess(t *testing.T) {
t.Parallel()
fc := forge.NewFakeClient()
// Seed FakeClient so methods return without errors.
fc.FileContents = map[string][]byte{
"org/repo/dummy.yaml": []byte("content"),
}
d := New(fc) // construct the real driver type
ctx := context.Background()
const goroutines = 12
var wg sync.WaitGroup
for range goroutines {
wg.Add(1)
go func() {
defer wg.Done()
// Exercise representative methods concurrently.
_, _ = d.GetFileContent(ctx, "org", "repo", "dummy.yaml")
_ = d.CommitFile(ctx, "org", "repo", "path.txt", "msg", []byte("data"))
}()
}
wg.Wait()
}Convention: use 12 goroutines. This is high enough to trigger races reliably but low enough to avoid resource exhaustion in CI. See pkg/behaviourtest/drivers/scm/github/race_test.go for the canonical example.
Assertions inside goroutines: assert not require
Inside goroutines spawned by a test, use assert.XXX (testify/assert), not require.XXX (testify/require). require calls t.FailNow(), which calls runtime.Goexit(). Go's testing package documents that FailNow must be called from the goroutine running the test function, not from other goroutines — calling it from a spawned goroutine violates this contract and can silently mispass the test or crash the process. assert calls t.Errorf(), which is safe from any goroutine.
go func() {
defer wg.Done()
tok, err := d.AccessToken(ctx)
assert.NoError(t, err) // safe from any goroutine
assert.Equal(t, "expected", tok)
// require.NoError(t, err) — never use require inside a goroutine
}()This applies to all require functions (require.NoError, require.Equal, require.NotNil, etc.) — the entire require package uses FailNow internally. The test goroutine itself (the function passed to t.Run or the Test* function) can use require normally.
Why synthetic stubs don't work
Stubs that implement an interface with no-ops or stateless pass-throughs hold no mutable state, so the race detector has nothing to detect. Even stubs that use atomic.Int64 counters are invisible to -race because atomics are correctly synchronized by definition. The point of a race test is to exercise the real type's fields — only a real constructor backed by a thread-safe fake can trigger the detector on unsynchronized production code.
Concurrent error handling
When goroutines fan out to perform independent operations that can each fail on their own (e.g., a sync.WaitGroup + go func loop creating providers), collect every failure and surface them together with errors.Join. Do not keep a single error variable (e.g., firstErr) that discards all failures after the first — that forces the user into fix-and-rerun cycles: they fix the one error shown, rerun, and only then discover the next.
The canonical pattern — a mutex-guarded []error accumulated across goroutines and joined after wg.Wait() — is in internal/cli/run.go (provider fan-out):
var (
mu sync.Mutex
wg sync.WaitGroup
errs []error
)
for _, pd := range allDefs {
wg.Add(1)
go func(pd harness.ProviderDef) {
defer wg.Done()
if err := sandbox.EnsureProvider(ctx, /* … */); err != nil {
mu.Lock()
errs = append(errs, fmt.Errorf("ensuring provider %q: %w", pd.Name, err))
mu.Unlock()
return
}
}(pd)
}
wg.Wait()
if err := errors.Join(errs...); err != nil {
return err
}The sync.Mutex is the idiom used here; writing each goroutine's result into its own pre-sized slice slot (one index per goroutine, no lock) is equally acceptable. The requirement is that no failure is dropped — not the specific synchronization mechanism.
This applies only to independent fan-out. When goroutines are not independent — you deliberately want the first failure to cancel the rest (e.g., an errgroup.Group sharing a context.Context) — fail-fast is correct and must not be forced into error collection.
When reviewing PRs: Flag a fan-out that captures only the first error (a single firstErr/err variable, or break-on-first) across independent goroutines as a medium-severity finding, and recommend collecting a []error and returning errors.Join. Do not flag intentional fail-fast cancellation patterns.
Context-aware blocking
Functions that accept context.Context must not use time.Sleep or other unconditionally-blocking calls. Use select to respect cancellation:
// Good — respects context cancellation.
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(backoff):
}
// Bad — blocks unconditionally, ignores cancellation.
time.Sleep(backoff)This applies to retry loops, polling intervals, and any deliberate delay. For retry patterns specifically, check whether the package already provides an injectable sleep function (e.g., sandbox.RetrySleepFn) for testability.
For blocking syscalls like syscall.Flock(LOCK_EX) that cannot be interrupted, add a comment documenting the worst-case blocking duration and why it is acceptable.
Error handling and naming conventions
Use typed constants over string literals
When the codebase defines constants for a value, use them instead of repeating string literals. For example, repos.ForgeGitHub and repos.ForgeGitLab (defined in internal/repos/manifest.go) should be used wherever a forge type is compared or assigned — not the raw strings "github" or "gitlab".
Before introducing a string literal for a domain value, search for existing constants:
grep -rn 'const.*Forge' internal/The same applies to credential modes (repos.CredModeWIF), tracker types, and other enumerated values. Using typed constants avoids silent breakage when a value is renamed and makes it clear which values are valid.
Prefer sentinel errors for programmatic error checking
When callers need to distinguish error conditions (e.g., "token not found" vs "unsupported forge"), define a package-level sentinel error and check it with errors.Is:
// Package-level sentinel — unexported, starts with "err".
var errGitLabTokenMissing = errors.New("no GitLab token found: set GITLAB_TOKEN or pass --gitlab-token")
func resolveGitLabToken() (string, error) {
if token := os.Getenv("GITLAB_TOKEN"); token != "" {
return token, nil
}
return "", errGitLabTokenMissing
}
// Caller checks the sentinel:
token, err := resolveGitLabToken()
if errors.Is(err, errGitLabTokenMissing) {
// handle missing token specifically
}Do not match errors by substring: strings.Contains(err.Error(), "token") couples error handling to message wording and breaks when messages change. Use errors.Is or errors.As for all programmatic error checks.
See internal/cli/forge_client.go (errGitLabTokenMissing), internal/cli/admin.go (errMintNotFound), and internal/cli/lock.go (errHarnessNotFound) for examples of this pattern in the codebase.
Use %q for values in error messages
Format user-provided or enumerated values with %q so they are consistently quoted in output. This makes error messages unambiguous when values contain spaces or are empty:
// Good — %q adds quotes automatically.
return fmt.Errorf("unsupported forge %q", forgeName)
// Bad — manual escaping is inconsistent and easy to forget.
return fmt.Errorf("unsupported forge \"%s\"", forgeName)This matches the pattern in internal/cli/tracker_client.go and internal/cli/forge_client.go.
Consistent error message content
When multiple code paths produce errors for the same condition across different forges or providers, ensure they mention the same remediation options. For example, if one "no token found" error suggests both the environment variable and the --token flag, other forge-specific token errors should do the same — so users see consistent guidance regardless of which code path triggers.
Go pitfalls
Timeout() bool interface and context.DeadlineExceeded
context.DeadlineExceeded implements interface{ Timeout() bool } and returns true. This means any timeout detection that uses an interface type assertion will incorrectly classify context deadline errors as timeouts:
// WRONG — matches context.DeadlineExceeded, which is not a transient
// network timeout but an intentional cancellation by the caller.
var te interface{ Timeout() bool }
if errors.As(err, &te) && te.Timeout() {
return true // retries context deadlines — incorrect
}Context deadline and cancellation errors represent intentional cancellation by the caller (e.g., a request timeout set by the application, a user-initiated cancel). They should never be classified as transient or retried — the caller chose to stop waiting, and retrying re-creates the same deadline.
Always guard against context errors before checking Timeout():
// CORRECT — context errors are excluded before the Timeout() check.
if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
return false
}
var te interface{ Timeout() bool }
if errors.As(err, &te) && te.Timeout() {
return true // only matches genuine network timeouts (e.g. net/http.Client.Timeout)
}See forge.IsTransient for the canonical example of the correct pattern.
When reviewing PRs: Flag any Timeout() bool interface assertion without a preceding errors.Is(err, context.DeadlineExceeded) guard as a medium-severity finding. The fix is to add the context-error check before the Timeout() check.
Template map iteration
Go's text/template range action visits map keys of basic types (string, int, uint, float) in sorted order — unlike bare range over a map in Go code. Do not flag {{ range $k, $v := .SomeMap }} in templates as non-deterministic when the key type is a basic type. See text/template documentation (search "sorted key order").
When reviewing PRs: Do not flag range over a basic-type-keyed map inside a text/template as non-deterministic output. The text/template package guarantees sorted iteration for string, int, uint, and float keys. This is a well-documented exception to Go's general rule that map iteration order is unspecified.
Injectable function variables (test seams)
Package-level variables that hold function values for test overriding must:
- Use an
XxxFnsuffix (e.g.,BuildWASMFn,RetrySleepFn,WranglerWhoamiFn). Both exported (XxxFn) and unexported (xxxFn) variables follow this suffix pattern - Default to the real implementation
- Include a doc comment following Go convention (starting with the variable name) that contains an "Override in tests to..." sentence describing the override behavior
- Be restored in a
t.Cleanupcallback when overridden
Examples: internal/sandbox/sandbox.go (RetrySleepFn), internal/dispatch/cf/provisioner.go (BuildWASMFn, CopyWASMExecFn).
Secure HTTP clients
Scope. These requirements apply to any client whose target URL is derived from untrusted configuration, user input, or remote content — that is where SSRF lives. A client that talks to a fixed first-party endpoint supplied by the sandbox/runner bootstrap is out of scope for the SSRF hardening below. For example, internal/cli/fetchskill.go POSTs to the runner-side fetch service on the host, which is bound to 0.0.0.0 per ADR 0046, bearer-token authenticated, and reached over a private address. The runner injects FULLSEND_FETCH_URL and FULLSEND_FETCH_TOKEN as reserved keys that harness YAML cannot shadow, so this is a deliberate trusted channel rather than an arbitrary configuration URL. Such clients should still set a timeout and bound the response body, but do not need IP filtering, proxy disabling, or an HTTPS-only rule. The current fetchskill.go client sets a timeout but does not yet bound its decoded response body; that gap is not an example to copy.
Prefer the shared, SSRF-hardened fetcher internal/fetch.FetchURL over constructing a raw http.Client. Pass it a fetch.FetchPolicy (see fetch.DefaultPolicy for the GitHub-content defaults) rather than re-implementing the protections. internal/fetch/fetch.go is the canonical reference.
FetchURL has a deliberately narrow envelope — reach for it only when all of these hold, otherwise it will reject the request or can't express what you need:
- The legitimate host set is known up front.
FetchURLrequires a non-emptyAllowedDomainsallowlist — an empty allowlist rejects every URL (isAllowedDomainreturns false), so the allowlist is mandatory, not optional. - GET, HTTP 200, no custom headers. It issues a
GETand sends no request headers — so it cannot carry authentication or use another method. Transient HTTP status codes (429, 502, 503) are retried with exponential backoff, but the function ultimately requires a 200 response. - Whole body buffered, port 443. It reads the entire body into memory and defaults
AllowedPortsto{"443"}.
When the fetch falls outside that envelope you must build a custom client. Common reasons: the legitimate host set is not knowable up front (e.g. internal/repos/manifest.go's LoadManifest accepts any user-supplied https:// host, so no allowlist covers it — this is why fetchManifestURL/safeDialContext exist and deliberately do not use FetchURL), authenticated requests, non-GET methods, non-200 handling, streaming, or a client reused across many calls. The complete worked pattern is LoadManifest's initial HTTPS-only gate together with fetchManifestURL/safeDialContext; the fetch functions enforce the remaining controls and HTTPS-only redirects, but do not independently reject a non-HTTPS initial URL. A custom in-scope client must apply all of these properties:
- HTTPS only. Reject
http://inputs, and validate the scheme on redirects too — aCheckRedirectthat lets anhttps://origin bounce tohttp://reopens the hole. - Reject internal/reserved IPs, on every connection. Inside a custom
DialContext, resolve the host the transport actually asks for (split theaddrit passes you, resolve it, check every IP withnetutil.CheckIP/netutil.IsInternal), then dial one of those validated IPs — never re-dial the hostname. Validating and dialing the exact same IP is the DNS-rebinding defense. Do this per connection, the waysafeDialContextdoes, so a redirect to a different host is resolved and validated before that connection.fetch.FetchURL's single up-front resolution and pinning is safe because it blocks redirects outright (itsCheckRedirectreturnshttp.ErrUseLastResponse); it cannot validate a redirect to a different host, so never copy that pattern into a client that follows redirects. - Keep the original hostname on the request. Only the dial address changes to the validated IP — the
http.RequestURL, and therefore SNI and certificate verification, must still carry the original hostname, or TLS verification breaks. Build the request from the original URL and letDialContextswap the address, exactly asfetch.FetchURLdoes (http.NewRequestWithContext(ctx, …, rawURL, …)while itsDialContextdials the IP). Never rewrite the request URL to the IP, and never reach forInsecureSkipVerifyto paper over the resulting cert failure. - Disable proxies. Set
Transport.Proxy = nilsoHTTP(S)_PROXYenv vars can't redirect the request. - Bound the time. Set an explicit timeout (30s is the repo default) via
context.WithTimeoutand/orhttp.Client.Timeout. - Bound the size. Wrap the response body in
io.LimitReader(body, max+1)and error if the read exceedsmax(1 MB is the manifest default; pick a limit appropriate to the payload). Neverio.ReadAll— orjson.NewDecoder— an unbounded body. - Constrain redirects. Block them (as
fetch.FetchURLdoes) or cap the hop count; and if you allow any hop, re-run the checks above against each redirect target — both the scheme check and, via per-connection dialing, the internal-IP check — not just the scheme.
When the set of legitimate hosts is known, add a domain allowlist too, even on the custom path.
Credential redaction for external content
Any runner feature that processes external content — validation script output, CI logs, script stdout/stderr — for injection into LLM prompts, logging, or file storage must redact credentials before that content leaves the runner boundary. The validation loop's redactFeedback function in internal/cli/run.go is the canonical implementation.
Invariants
Scan
RunnerEnvfor credential literal values. Iterate the runner environment map and replace every value whose key is classified as sensitive bysensitiveEnvKey(explicit names likePUSH_TOKEN,GH_TOKEN, plus suffix matches on_TOKEN,_SECRET,_PASSWORD,_KEY,_CREDENTIALS) with[REDACTED:<key>]. Skip values shorter thanminRedactableSecretLen(currently 8) — short values like"main"or"true"cause false-positive mangling.Apply
security.SecretRedactoras a second-pass fallback. TheRunnerEnvscan only catches credentials the harness declared. Asecurity.NewSecretRedactor().Scan(content)call catches credentials with recognizable shapes (known-prefix tokens such asghp_,sk-ant-,AKIA, PEM blocks, connection strings) that never passed through the runner environment — for example, a key baked into a test fixture or a pre-commit hook printing its own secrets.Use
truncateUTF8when enforcing size limits on external content. Naive byte slicing (s[:max]) can split a multi-byte UTF-8 rune, producing invalid text that breaks downstream JSON serialization or LLM tokenization. UsetruncateUTF8(s, max)(defined ininternal/cli/run.go), which backs up to the last valid rune boundary before appending a[truncated]marker.Write files containing potential secrets with mode
0600. Feedback files, redacted logs, and any file derived from external content must useos.WriteFile(path, data, 0o600)— not0644. The run directory is uploaded as a CI artifact; restrictive permissions limit exposure if the artifact is downloaded to a shared filesystem.
Why both passes are needed
Neither pass alone is sufficient. Opaque tokens (e.g., a GitHub installation token with no recognizable prefix) have no pattern for the SecretRedactor to match — only the literal RunnerEnv scan catches those. Conversely, credentials that never entered the runner environment (a PEM key printed by a repo hook, a fixture secret) are invisible to the env scan — only the pattern-based SecretRedactor catches those.
When this applies
Apply these invariants whenever external content crosses a trust boundary in the runner:
- Validation script output injected into the next iteration's LLM prompt (
feedback_mode) - Pre-commit or post-script output routed back to the agent
- CI log fragments stored in the run directory
- Any new feature that captures subprocess output for prompt injection, storage, or logging
See also #2107 (replicate existing security patterns) and #2872 (post-script security invariants) for related guidance in other layers.
Running the fullsend CLI
Audience: contributors and agents working from a repo checkout. Do not change end-user or operator guides under docs/guides/getting-started/, docs/guides/user/, or docs/guides/infrastructure/ to require go run — those audiences install a released fullsend binary.
When agents (or humans working from this checkout) need the fullsend CLI, invoke it from the repo root with:
go run ./cmd/fullsend <subcommand> …Do not use a preinstalled fullsend from mise, $PATH, GOBIN, go install, or another clone. Those binaries often lag the branch you are on. Enrollment and other mint-mutating commands rewrite Cloud Run env vars; a stale CLI can apply obsolete merge logic against the hosted mint.
This already happened: an enrollment for crc-org/crc used a June-era mise fullsend that still wrote org-scoped ROLE_APP_IDS keys and re-derived ALLOWED_ROLES from slash-keyed entries only. Shared role-only keys such as e2e and fix were ignored, so the mint dropped those roles from ALLOWED_ROLES and e2e broke until the mint was restored. Current tree enroll is safer, but the durable fix for agents is to always run the CLI from source.
make go-build / bin/fullsend is fine when you intentionally build this checkout first. Prefer go run unless you have a reason to keep a built binary.
Running e2e tests
The e2e tests mint short-lived GitHub App installation tokens via the central token mint. Pool-org admin operations use mint/OIDC in CI and do not require a dedicated mint URL secret.
- CI (mint): Uses the hosted public mint (same default as
fullsend admin --mint-url) with the workflow's OIDC identity. The e2e workflow exchanges the OIDC JWT for ane2e-role installation token on the pool org. Override withFULLSEND_MINT_URLif needed. - Local: Run
gh auth login(or setGH_TOKEN/GITHUB_TOKENwith pool-org admin access). Mint usesFULLSEND_MINT_URLor the hosted default.
Do not increase e2e or behaviour suite timeouts without explicit human authorization in the current session (or an issue/PR comment that clearly authorizes that bump). Suite ceilings are job timeout-minutes in .github/workflows/e2e.yml for the e2e / behaviour jobs, the matching go test -timeout values in the e2e-test / behaviour-test Make targets, and the default E2E_LOCK_TIMEOUT. Scenario-level wait or assertion windows (for example dispatch detection) are out of scope when an issue asks for them — those are not suite ceilings. On timeout failures, diagnose and fix the root cause (slow scenarios, unnecessary waits, lock contention); do not open a PR whose primary change is raising the suite timeout.
When reviewing PRs: Flag unauthorized suite-timeout increases as an important-severity finding (policy violation). Explicit human authorization in the linked issue or a PR comment is the only exception.
See docs/guides/dev/e2e-testing.md and make help for pool org setup and troubleshooting.
Per-repo config field checklist
When adding a new field to perRepoConfig (internal/config/config.go) that needs validation, you must wire validation into both the write path and the run path. The two paths use different validation entry points, and missing either one creates a gap.
Why two validation paths exist
perRepoConfig.Validate() runs on write paths — for example, when fullsend config set persists a config file. However, fullsend run loads config via loadPerRepoLayers() (in internal/config/interfaces.go), which does not call Validate(). This means validation logic that only lives in Validate() never fires when the config is consumed at runtime. Invalid entries (unknown keys, bad references) are silently accepted.
The codebase solves this with a dual-validation pattern: Validate() covers write paths, and inline validation in runAgent() (internal/cli/run.go) covers the run path.
Canonical examples
agentSettings()(internal/cli/run.go) — validates agent entries loaded from config before applying them. The function's doc comment explicitly states: "fullsend runnever callsValidate()on the config it loads, so this is where those values get checked."ValidateModelAliases()(internal/config/config.go) — exported validation function called both inValidate()(write path) and directly inrunAgent()(run path) to reject unknown alias keys and invalid model references before the sandbox is created.run_models_aliases_test.go(internal/cli/) — run-path integration test verifying that invalidmodels.aliasesvalues are rejected byrunAgent()before sandbox creation.
Checklist
When adding a new validated field to perRepoConfig:
- Add validation in
Validate()— this covers write paths (e.g.,fullsend config set). - Add inline validation in
runAgent()— follow theagentSettings()/ValidateModelAliases()pattern: validate the effective (merged) value before the sandbox is created, not after. If the validation logic is non-trivial, export it as a standalone function (likeValidateModelAliases) so both call sites use the same logic. - Add a run-path integration test — follow the pattern in
run_models_aliases_test.go: write an invalid config, callrunAgent(), and assert that it fails with the expected error before any sandbox is created.
When reviewing PRs
When reviewing a PR that adds a new validated field to perRepoConfig, check that validation fires on the run path — not only in Validate(). Flag a missing run-path validation call as a medium-severity finding. The fix is to add inline validation in runAgent() and a corresponding integration test.
