feat(auth): decouple auth pipeline from harness-specific Go code (Phases 1-2) by ptone · Pull Request #516 · GoogleCloudPlatform/scion

ptone · 2026-06-28T05:13:34Z

Summary

Implements Phases 1 and 2 of issue 311 — decouples the auth pipeline from harness-specific Go code and fixes hydration ordering.

Phase 1: Fix hydration ordering

Harness-config hydration now runs before extractRequiredEnvKeys in the env-gather (HTTP 202) path
Hub-managed configs are downloaded with a 10-second timeout and graceful fallback to on-disk
extractRequiredEnvKeys accepts an optional hydrated path to supplement on-disk search

Phase 2: Generic env var passthrough

Added EnvVars map[string]string to AuthConfig for config-driven auth env vars
GatherAuthWithEnv accepts *config.HarnessAuthMetadata and populates EnvVars from declared required_env keys
ResolveAuth (both ContainerScriptHarness and Generic) forwards auth.EnvVars into container env (hardcoded fields take precedence)
isAuthEnvKey made config-aware via variadic extraAuthKeys parameter
filterResolvedSecretsForResolvedAuth and isAuthCandidateSecret extended with config-driven key awareness

What this enables

New harnesses (e.g. GitHub Copilot CLI) can declare auth requirements in their config.yaml auth: block and have tokens flow through the pipeline without any harness-specific Go code:

auth:
  types:
    api-key:
      required_env:
        any_of: [COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN]

What this does NOT change

All existing hardcoded auth paths remain functional — these changes are purely additive
Phases 3-4 (migrating built-in harnesses to config-driven auth and removing hardcoded fields) are separate follow-up work

Files changed (8 files, +403/-22)

File	Change
`pkg/api/types.go`	Added `EnvVars map[string]string` to `AuthConfig`
`pkg/harness/auth.go`	`GatherAuthWithEnv` accepts auth metadata; `gatherConfigEnvVars` helper
`pkg/harness/auth_test.go`	6 new tests (copilot scenario, overlay, broker mode, nil/empty metadata)
`pkg/harness/container_script_harness.go`	`ResolveAuth` forwards `auth.EnvVars`
`pkg/harness/generic.go`	`ResolveAuth` forwards `auth.EnvVars`
`pkg/agent/run.go`	Config-aware `isAuthEnvKey`, `configAuthEnvKeySet`, auth metadata resolution
`pkg/agent/run_test.go`	4 new tests (isAuthEnvKey, configAuthEnvKeySet, filter with config keys)
`pkg/runtimebroker/handlers.go`	Hydration before env-gather; `extractRequiredEnvKeys` accepts hydrated path

Test plan

pkg/harness/... tests pass (0.156s) — 6 new config-driven auth tests
pkg/agent/... tests pass (4.870s) — 4 new tests
pkg/runtimebroker/... tests pass (2.556s)
pkg/api/... tests pass (0.009s)
Full go build ./... clean
Verify copilot harness auth flow end-to-end with hub-managed config

Closes issue 311 (Phases 1-2)

Implement Phases 1 and 2 of the auth pipeline decoupling: Phase 1 - Fix hydration ordering: - Add harness-config hydration to the env-gather pre-check path in handlers.go, before extractRequiredEnvKeys runs - Hub-managed harness-configs are now downloaded (with 10s timeout and graceful fallback) so config-driven auth metadata is available when the broker determines which env vars to request from the CLI - extractRequiredEnvKeys falls back to the hydrated path when on-disk search doesn't find the config Phase 2 - Generic env var passthrough in AuthConfig: - Add EnvVars map[string]string to api.AuthConfig for config-driven auth env vars - Modify GatherAuthWithEnv to accept optional *config.HarnessAuthMetadata and populate EnvVars from declared required_env keys - Modify ContainerScriptHarness.ResolveAuth and Generic.ResolveAuth to forward auth.EnvVars into the container environment - Make isAuthEnvKey config-aware via optional extraAuthKeys parameter - Update filterResolvedSecretsForResolvedAuth to recognize config-driven auth keys This is additive — existing hardcoded paths continue working for all built-in harnesses. The copilot harness's COPILOT_GITHUB_TOKEN / GH_TOKEN / GITHUB_TOKEN now flows through the auth pipeline via config alone, without any copilot-specific Go code.

google-cla · 2026-06-28T05:13:45Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request introduces config-driven auth metadata handling, allowing harness configurations to declare required environment variables that flow through the auth pipeline without hardcoded Go fields. The review feedback highlights a correctness issue where template-bundled harness configs fail to resolve because templatePaths is omitted during directory resolution. Additionally, the feedback suggests refactoring redundant directory resolutions and replacing variadic parameters (extraAuthKeys and hydratedHarnessConfigPath) with standard map and string parameters to avoid unnecessary slice allocations and adhere to idiomatic Go patterns.

gemini-code-assist · 2026-06-28T05:15:24Z

+	if harnessConfigName != "" {
+		if hcDir, err := resolveHarnessConfigDir(ctx, harnessConfigName, projectDir); err == nil && hcDir.Config.Auth != nil {
+			authMeta = hcDir.Config.Auth
+		}


In Start(), resolveHarnessConfigDir is called without templatePaths. However, earlier in Start() (around line 243), it is called with templatePaths... because harness configs can be bundled inside templates (per §3.4 of agnostic-template-design). By omitting templatePaths here, any template-bundled harness configs will fail to resolve, leaving authMeta as nil.\n\nAdditionally, resolving the harness config directory twice from the filesystem/network is inefficient. Consider promoting hcDir (or authMeta) to the outer scope of Start() where it is first resolved, and reusing it here to avoid both the correctness bug and the redundant resolution.

gemini-code-assist · 2026-06-28T05:15:24Z

 }

-func isAuthEnvKey(key string) bool {
+func isAuthEnvKey(key string, extraAuthKeys ...map[string]struct{}) bool {


Using a variadic parameter extraAuthKeys ...map[string]struct{} causes Go to allocate a new slice on every call to isAuthEnvKey when arguments are passed. Since isAuthEnvKey is called inside hot loops (e.g., when filtering environment variables and secrets), this can lead to unnecessary allocations and garbage collection pressure.\n\nSince we only ever pass a single configKeys map, we should change the parameter to a regular map[string]struct{} (which can be nil).

Suggested change

func isAuthEnvKey(key string, extraAuthKeys ...map[string]struct{}) bool {

func isAuthEnvKey(key string, extraAuthKeys map[string]struct{}) bool {

gemini-code-assist · 2026-06-28T05:15:24Z

+		for _, extra := range extraAuthKeys {
+			if _, ok := extra[key]; ok {
+				return true
+			}
+		}
 		return false


Update the default case to check the single extraAuthKeys map directly instead of iterating over a variadic slice.

\t\tif extraAuthKeys != nil {\n\t\t\tif _, ok := extraAuthKeys[key]; ok {\n\t\t\t\treturn true\n\t\t\t}\n\t\t}\n\t\treturn false

gemini-code-assist · 2026-06-28T05:15:24Z

+	for _, key := range builtins {
+		if !isAuthEnvKey(key) {
+			t.Errorf("isAuthEnvKey(%q) = false, want true", key)
+		}
+	}
+	if isAuthEnvKey("RANDOM_ENV_VAR") {
+		t.Error("isAuthEnvKey(RANDOM_ENV_VAR) = true, want false")
+	}


Update the test calls to pass nil for extraAuthKeys to match the non-variadic signature of isAuthEnvKey.

\tfor _, key := range builtins {\n\t\tif !isAuthEnvKey(key, nil) {\n\t\t\tt.Errorf(\"isAuthEnvKey(%q) = false, want true\", key)\n\t\t}\n\t}\n\tif isAuthEnvKey(\"RANDOM_ENV_VAR\", nil) {

gemini-code-assist · 2026-06-28T05:15:24Z

+// config directory that supplements the on-disk search. This allows env-gather
+// to see auth metadata from hub-managed harness-configs that haven't been
+// downloaded to the standard on-disk locations yet.
+func (s *Server) extractRequiredEnvKeys(req CreateAgentRequest, hydratedHarnessConfigPath ...string) ([]string, map[string]api.SecretKeyInfo) {


Using a variadic parameter hydratedHarnessConfigPath ...string to represent an optional single string is a minor anti-pattern in Go. It is more idiomatic and type-safe to pass a regular string parameter (which can be empty "").

Suggested change

func (s *Server) extractRequiredEnvKeys(req CreateAgentRequest, hydratedHarnessConfigPath ...string) ([]string, map[string]api.SecretKeyInfo) {

func (s *Server) extractRequiredEnvKeys(req CreateAgentRequest, hydratedHarnessConfigPath string) ([]string, map[string]api.SecretKeyInfo) {

gemini-code-assist · 2026-06-28T05:15:25Z


+		// Fall back to hydrated hub-managed harness-config when on-disk
+		// search didn't find the config (or didn't populate auth metadata).
+		if harnessType == "" && len(hydratedHarnessConfigPath) > 0 && hydratedHarnessConfigPath[0] != "" {


Simplify the check to use the regular string parameter directly.

\t\tif harnessType == \"\" && hydratedHarnessConfigPath != \"\" {

- Fix gofmt: remove extra blank line in auth_test.go - Fix duplicate config resolution: reuse harness config dir already resolved during image resolution instead of re-resolving without templatePaths - Fix nil safety: add nil check on hcDir in hydrated config fallback in handlers.go

The SCION_TEST_UNSET_TOKEN key doesn't exist in the test environment, so there's nothing to unset. The assertion already verifies that a nonexistent key doesn't appear in EnvVars.

gemini-code-assist Bot reviewed Jun 28, 2026

View reviewed changes

ptone mentioned this pull request Jun 28, 2026

Decouple auth pipeline from harness-specific Go code; fix hydration ordering ptone/scion#311

Open

Scion Agent (hi-dev) added 2 commits June 28, 2026 05:24

fix(auth): remove unchecked os.Unsetenv call in test

7c803d9

The SCION_TEST_UNSET_TOKEN key doesn't exist in the test environment, so there's nothing to unset. The assertion already verifies that a nonexistent key doesn't appear in EnvVars.

ptone merged commit a2acb60 into GoogleCloudPlatform:main Jun 28, 2026
7 of 9 checks passed

ptone deleted the scion/harness-auth-pipeline-fix branch June 28, 2026 12:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(auth): decouple auth pipeline from harness-specific Go code (Phases 1-2)#516

feat(auth): decouple auth pipeline from harness-specific Go code (Phases 1-2)#516
ptone merged 3 commits into
GoogleCloudPlatform:mainfrom
ptone:scion/harness-auth-pipeline-fix

ptone commented Jun 28, 2026

Uh oh!

google-cla Bot commented Jun 28, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	func isAuthEnvKey(key string, extraAuthKeys ...map[string]struct{}) bool {
	func isAuthEnvKey(key string, extraAuthKeys map[string]struct{}) bool {

	func (s *Server) extractRequiredEnvKeys(req CreateAgentRequest, hydratedHarnessConfigPath ...string) ([]string, map[string]api.SecretKeyInfo) {
	func (s *Server) extractRequiredEnvKeys(req CreateAgentRequest, hydratedHarnessConfigPath string) ([]string, map[string]api.SecretKeyInfo) {

Uh oh!

Conversation

ptone commented Jun 28, 2026

Summary

Phase 1: Fix hydration ordering

Phase 2: Generic env var passthrough

What this enables

What this does NOT change

Files changed (8 files, +403/-22)

Test plan

Uh oh!

google-cla Bot commented Jun 28, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant