mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(agent): wire updateCommandEnv into process manager (#22451)
## Problem The `agentproc` process manager spawns processes with only `os.Environ()`, missing agent-level environment variables like `GIT_ASKPASS`, `CODER_*`, and `GIT_SSH_COMMAND` that are injected by the agent's `updateCommandEnv` function. This means processes started through the HTTP process API (used by chat tools) cannot authenticate git operations via the Coder gitaskpass helper. By contrast, SSH sessions get the full agent environment because the SSH server calls `updateCommandEnv` via its `UpdateEnv` config hook. ## Fix Wire the agent's `updateCommandEnv` hook into the process manager so all spawned processes receive the full agent environment. The hook is: - Passed as a parameter through `NewAPI` → `newManager` - Called in `manager.start()` with `os.Environ()` as the base, producing the same enriched env that SSH sessions get - Gracefully falls back to `os.Environ()` if the hook returns an error Request-level env vars (`req.Env`, set by chat tools) are still appended last and take precedence. ## Changes - `agent/agentproc/process.go`: Add `updateEnv` field to manager, call it when building process env - `agent/agentproc/api.go`: Accept `updateEnv` parameter in `NewAPI` - `agent/agent.go`: Pass `a.updateCommandEnv` when creating the process API - `agent/agentproc/api_test.go`: Add `UpdateEnvHook` and `UpdateEnvHookOverriddenByReqEnv` tests Co-authored-by: Coder <coder@coder.com>
This commit is contained in:
@@ -88,11 +88,18 @@ func postSignal(t *testing.T, handler http.Handler, id string, req workspacesdk.
|
||||
// execer, returning the handler and API.
|
||||
func newTestAPI(t *testing.T) http.Handler {
|
||||
t.Helper()
|
||||
return newTestAPIWithUpdateEnv(t, nil)
|
||||
}
|
||||
|
||||
// newTestAPIWithUpdateEnv creates a new API with an optional
|
||||
// updateEnv hook for testing environment injection.
|
||||
func newTestAPIWithUpdateEnv(t *testing.T, updateEnv func([]string) ([]string, error)) http.Handler {
|
||||
t.Helper()
|
||||
|
||||
logger := slogtest.Make(t, &slogtest.Options{
|
||||
IgnoreErrors: true,
|
||||
}).Leveled(slog.LevelDebug)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv)
|
||||
t.Cleanup(func() {
|
||||
_ = api.Close()
|
||||
})
|
||||
@@ -257,6 +264,54 @@ func TestStartProcess(t *testing.T) {
|
||||
require.Equal(t, 0, *resp.ExitCode)
|
||||
require.Contains(t, strings.TrimSpace(resp.Output), envVal)
|
||||
})
|
||||
|
||||
t.Run("UpdateEnvHook", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
envKey := fmt.Sprintf("TEST_UPDATE_ENV_%d", time.Now().UnixNano())
|
||||
envVal := "injected_by_hook"
|
||||
|
||||
handler := newTestAPIWithUpdateEnv(t, func(current []string) ([]string, error) {
|
||||
return append(current, fmt.Sprintf("%s=%s", envKey, envVal)), nil
|
||||
})
|
||||
|
||||
// The process should see the variable even though it
|
||||
// was not passed in req.Env.
|
||||
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
|
||||
Command: fmt.Sprintf("printenv %s", envKey),
|
||||
})
|
||||
|
||||
resp := waitForExit(t, handler, id)
|
||||
require.NotNil(t, resp.ExitCode)
|
||||
require.Equal(t, 0, *resp.ExitCode)
|
||||
require.Contains(t, strings.TrimSpace(resp.Output), envVal)
|
||||
})
|
||||
|
||||
t.Run("UpdateEnvHookOverriddenByReqEnv", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
envKey := fmt.Sprintf("TEST_OVERRIDE_%d", time.Now().UnixNano())
|
||||
hookVal := "from_hook"
|
||||
reqVal := "from_request"
|
||||
|
||||
handler := newTestAPIWithUpdateEnv(t, func(current []string) ([]string, error) {
|
||||
return append(current, fmt.Sprintf("%s=%s", envKey, hookVal)), nil
|
||||
})
|
||||
|
||||
// req.Env should take precedence over the hook.
|
||||
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
|
||||
Command: fmt.Sprintf("printenv %s", envKey),
|
||||
Env: map[string]string{envKey: reqVal},
|
||||
})
|
||||
|
||||
resp := waitForExit(t, handler, id)
|
||||
require.NotNil(t, resp.ExitCode)
|
||||
require.Equal(t, 0, *resp.ExitCode)
|
||||
// When duplicate env vars exist, shells use the last
|
||||
// value. Since req.Env is appended after the hook,
|
||||
// the request value wins.
|
||||
require.Contains(t, strings.TrimSpace(resp.Output), reqVal)
|
||||
})
|
||||
}
|
||||
|
||||
func TestListProcesses(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user