From a03de32300f9d258e69a0130103a07e8861fedff Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Tue, 2 Jun 2026 22:02:12 +0530 Subject: [PATCH 1/2] fix(stack): redact env in PATCH /stacks/:slug/env response + panic-safe audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bug-bash (2026-06-02) findings on the stack env-update handler: - The response echoed the full merged env map verbatim (`"env": merged`), unlike DeployHandler.UpdateEnv which redacts the same shape. A PATCH setting one harmless key returns ALL previously-set values — including credential URLs and secret-keyed values carried over in stacks.env_vars — in cleartext into proxy logs / browser panels / agent transcripts. Now returns redactEnvVars(merged) (stored value stays unredacted; only the response is masked). - The audit-emit goroutine was a bare `go func` with no panic recovery; switch to safego.Go to match runStackDeploy/runStackRedeploy. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/handlers/stack.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/internal/handlers/stack.go b/internal/handlers/stack.go index d2bc2c65..297cbfd0 100644 --- a/internal/handlers/stack.go +++ b/internal/handlers/stack.go @@ -1265,30 +1265,38 @@ func (h *StackHandler) UpdateEnv(c *fiber.Ctx) error { "keys_deleted": deletes, "total_after": len(merged), }) - go func(teamID uuid.UUID, stackID uuid.UUID, slug string, meta []byte) { + // safego.Go (not a bare `go func`) so a panic in the audit insert is + // recovered instead of crashing the worker goroutine / process — matches + // the runStackDeploy/runStackRedeploy launches in this file. + safego.Go("stack.env.audit", func() { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() if aErr := models.InsertAuditEvent(ctx, h.db, models.AuditEvent{ - TeamID: teamID, + TeamID: team.ID, Actor: auditActorSystem, Kind: "stack.env.updated", ResourceType: "stack", - ResourceID: uuid.NullUUID{UUID: stackID, Valid: true}, + ResourceID: uuid.NullUUID{UUID: stack.ID, Valid: true}, Summary: "updated env vars on stack " + slug + "", - Metadata: meta, + Metadata: auditMeta, }); aErr != nil { slog.Warn("stack.env.audit_failed", - "error", aErr, "team_id", teamID, "stack_id", stackID, "slug", slug) + "error", aErr, "team_id", team.ID, "stack_id", stack.ID, "slug", slug) } - }(team.ID, stack.ID, slug, auditMeta) + }) slog.Info("stack.env.updated", "slug", slug, "team_id", team.ID, "stack_id", stack.ID, "keys_set", len(body.Env)-deletes, "keys_deleted", deletes, "total_after", len(merged)) return c.JSON(fiber.Map{ - "ok": true, - "env": merged, + "ok": true, + // Redact outbound env vars — mirrors DeployHandler.UpdateEnv and + // GET /deploy/:id. The stored value (persisted above) is the + // unredacted merged map; only the response JSON is masked so + // secrets carried over from earlier PATCHes never echo in cleartext + // into proxy logs / agent transcripts. + "env": redactEnvVars(merged), "message": "Env vars persisted. Call POST /stacks/" + slug + "/redeploy to apply.", }) } From 172e5f189cbaae3937f1197182ec56de9ab04249 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Tue, 2 Jun 2026 22:27:29 +0530 Subject: [PATCH 2/2] test(stack): assert PATCH /env response redacts secret values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update TestStack_PatchEnv_PersistsAndReturns to expect the response to mask DATABASE_URL to "***" (key contains "URL") while NODE_ENV passes through — matching the bug-bash #1 redaction fix. The DB round-trip assertions still verify the STORED value is the unredacted "postgres://example". Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/handlers/stack_env_persist_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/handlers/stack_env_persist_test.go b/internal/handlers/stack_env_persist_test.go index fdb95d5d..003c25ce 100644 --- a/internal/handlers/stack_env_persist_test.go +++ b/internal/handlers/stack_env_persist_test.go @@ -119,10 +119,14 @@ func TestStack_PatchEnv_PersistsAndReturns(t *testing.T) { } require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) assert.True(t, body.OK) + // The RESPONSE redacts secret-bearing values (bug bash #1): DATABASE_URL + // (key contains "URL") is masked to "***", while a non-secret key like + // NODE_ENV passes through. The STORED value stays unredacted — asserted + // against the DB below. This mirrors DeployHandler.UpdateEnv. assert.Equal(t, map[string]string{ - "DATABASE_URL": "postgres://example", + "DATABASE_URL": "***", "NODE_ENV": "production", - }, body.Env, "response carries the merged env") + }, body.Env, "response masks secret values but keeps non-secret keys") // DB round-trip — pre-fix this would still be `{}` because the // handler dropped the payload. With migration 062 + UpdateStackEnvVars