From 8fe1f5e131b125ec0fb254fd69514d3a3ac6c085 Mon Sep 17 00:00:00 2001 From: chenchen Date: Wed, 10 Jun 2026 00:33:47 +0800 Subject: [PATCH] fix(agent,secret): enforce CRYPTO_SECRET for agent deploy (#31) and guard unscoped vault purge (#33) #31: HeicodeDeployAgent now refuses to deploy unless CRYPTO_SECRET is explicitly configured, so the per-agent access_token is sealed with a key that survives a container restart. common.CryptoSecret is never literally "" (defaults to uuid/SessionSecret), so the sealAgentToken plaintext fallback was effectively unreachable; the real hazard is an ephemeral random seal key making tokens undecryptable after restart. Dev-only override: HEICODE_ALLOW_PLAINTEXT_AGENT_TOKEN_IN_DEV=true. Verified prod container has CRYPTO_SECRET set (64 chars) -> deploy stays allowed. #33: StartSecretPurgeTask refuses to start a whole-vault purge when HEICODE_SECRET_PURGE_NAME_PREFIX is empty unless HEICODE_SECRET_PURGE_VAULT_EXCLUSIVE=true, so HM never permanently purges another tenant's soft-deleted secrets in a shared vault. Logs the resolved purge scope at startup. Both gates extracted into pure, unit-tested helpers (agentTokenSealKeyConfigured, secretPurgeScopeAllowed). Affects: Manager only (Agent deploy + Secret lifecycle). No Client/Swarm/billing/audit schema change. Co-Authored-By: Claude Opus 4.8 --- heicode/controller/agent_template_handlers.go | 32 +++++++++++++++++++ heicode/controller/agent_template_test.go | 10 ++++++ heicode/controller/secret_lifecycle.go | 30 +++++++++++++++++ heicode/controller/secret_lifecycle_test.go | 17 ++++++++++ 4 files changed, 89 insertions(+) diff --git a/heicode/controller/agent_template_handlers.go b/heicode/controller/agent_template_handlers.go index 03cdb1d..aa13b5d 100644 --- a/heicode/controller/agent_template_handlers.go +++ b/heicode/controller/agent_template_handlers.go @@ -3,6 +3,7 @@ package controller import ( "context" "crypto/subtle" + "os" "strconv" "strings" "sync" @@ -38,6 +39,23 @@ func sealAgentToken(token string) string { return sealed } +// agentTokenSealKeyConfigured reports whether a stable, explicitly-configured +// CRYPTO_SECRET is present so the per-agent access_token can be sealed with a +// key that survives a container restart (#31). +// +// Subtle but important: common.CryptoSecret is NEVER literally "" — it defaults +// to uuid.New() (constants.go) and, when CRYPTO_SECRET is unset, falls back to +// SessionSecret (init.go). So the "CryptoSecret unset?" plaintext fallback in +// sealAgentToken is effectively unreachable; the real production hazard is a +// key that is not stable across restarts. If CRYPTO_SECRET is not explicitly +// set, the seal key may be an ephemeral random UUID, so every previously sealed +// agent token becomes undecryptable after the next restart. We therefore gate +// agent deployment on CRYPTO_SECRET being explicitly configured. Pure helper so +// the policy is unit-testable without mutating process env globals. +func agentTokenSealKeyConfigured(cryptoSecretEnv string) bool { + return strings.TrimSpace(cryptoSecretEnv) != "" +} + // unsealAgentToken reverses sealAgentToken. If the stored value is not a sealed // blob (legacy plaintext / CryptoSecret unset), it is returned as-is. func unsealAgentToken(stored string) string { @@ -117,6 +135,20 @@ func HeicodeDeployAgent(c *gin.Context) { agentError(c, "POLICY_REJECTED", "authentication required") return } + + // #31: refuse to deploy when the per-agent access_token cannot be sealed with + // a stable key. Without an explicit CRYPTO_SECRET the seal key falls back to + // an ephemeral random value, so the sealed token becomes undecryptable after + // the next restart — the agent would silently lose its credential. A dev-only + // override keeps local runs (no CRYPTO_SECRET) working; it must never be set + // in production. + if !agentTokenSealKeyConfigured(os.Getenv("CRYPTO_SECRET")) && + !common.GetEnvOrDefaultBool("HEICODE_ALLOW_PLAINTEXT_AGENT_TOKEN_IN_DEV", false) { + common.SysLog("agent deploy rejected: CRYPTO_SECRET not set (per-agent access_token cannot be sealed with a restart-stable key)") + agentError(c, "POLICY_REJECTED", "agent deployment is disabled until CRYPTO_SECRET is configured, so the per-agent access token can be sealed with a key that survives restarts (dev-only override: HEICODE_ALLOW_PLAINTEXT_AGENT_TOKEN_IN_DEV=true)") + return + } + var req struct { TemplateID string `json:"template_id"` BindingIDs []int `json:"binding_ids"` diff --git a/heicode/controller/agent_template_test.go b/heicode/controller/agent_template_test.go index 837717e..7f622c0 100644 --- a/heicode/controller/agent_template_test.go +++ b/heicode/controller/agent_template_test.go @@ -264,3 +264,13 @@ func TestTemplateAgentResponse(t *testing.T) { require.Equal(t, []int{1, 2}, resp["binding_ids"]) require.Equal(t, "running", resp["status"]) } + +// #31: agent deployment requires an explicitly configured CRYPTO_SECRET so the +// per-agent access_token is sealed with a key that survives a container restart. +// common.CryptoSecret is never literally "" (it defaults to uuid/SessionSecret), +// so the gate must key off the CRYPTO_SECRET env value, not the runtime var. +func TestAgentTokenSealKeyConfigured(t *testing.T) { + require.False(t, agentTokenSealKeyConfigured("")) + require.False(t, agentTokenSealKeyConfigured(" ")) + require.True(t, agentTokenSealKeyConfigured("a-real-secret")) +} diff --git a/heicode/controller/secret_lifecycle.go b/heicode/controller/secret_lifecycle.go index 0399b26..406945a 100644 --- a/heicode/controller/secret_lifecycle.go +++ b/heicode/controller/secret_lifecycle.go @@ -214,6 +214,19 @@ func StartSecretPurgeTask() { if strings.TrimSpace(os.Getenv("AZURE_KEY_VAULT_URL")) == "" { return // no vault configured — nothing to purge } + // #33: a destructive purge that scans the WHOLE vault is only safe when + // HM owns the vault exclusively. If no name prefix scopes purging to + // HM-managed secrets, require an explicit opt-in (HEICODE_SECRET_PURGE_ + // VAULT_EXCLUSIVE=true) so HM never permanently purges another tenant's + // soft-deleted secrets that happen to live in a shared vault. + purgePrefix := strings.TrimSpace(common.GetEnvOrDefaultString("HEICODE_SECRET_PURGE_NAME_PREFIX", "")) + vaultExclusive := common.GetEnvOrDefaultBool("HEICODE_SECRET_PURGE_VAULT_EXCLUSIVE", false) + allowed, scopeDesc := secretPurgeScopeAllowed(purgePrefix, vaultExclusive) + if !allowed { + common.SysLog("secret purge task NOT started: " + scopeDesc) + return + } + common.SysLog("secret purge scope: " + scopeDesc) intervalHours := common.GetEnvOrDefault("HEICODE_SECRET_PURGE_INTERVAL_HOURS", 24) if intervalHours < 1 { intervalHours = 24 @@ -231,6 +244,23 @@ func StartSecretPurgeTask() { }) } +// secretPurgeScopeAllowed decides whether the destructive vault purge may run, +// given the configured name prefix and the vault-exclusive opt-in (#33). A purge +// that scans the WHOLE vault (empty prefix) is only safe when HM owns the vault +// exclusively, so it must be explicitly opted in. Returns the decision plus a +// human-readable scope/refusal description for the startup log. Pure helper for +// unit testing without touching process env. +func secretPurgeScopeAllowed(namePrefix string, vaultExclusive bool) (bool, string) { + prefix := strings.TrimSpace(namePrefix) + if prefix == "" { + if !vaultExclusive { + return false, "HEICODE_SECRET_PURGE_NAME_PREFIX is empty and HEICODE_SECRET_PURGE_VAULT_EXCLUSIVE!=true — refusing to purge an entire (possibly shared) vault. Set a name prefix to scope to HM-managed secrets, or set HEICODE_SECRET_PURGE_VAULT_EXCLUSIVE=true only if this vault is exclusive to HM." + } + return true, "ENTIRE vault (HEICODE_SECRET_PURGE_VAULT_EXCLUSIVE=true, no name prefix)" + } + return true, "secrets with name prefix '" + prefix + "'" +} + func secretPurgeRetentionDays() int { d := common.GetEnvOrDefault("HEICODE_SECRET_PURGE_RETENTION_DAYS", 30) if d < 1 { diff --git a/heicode/controller/secret_lifecycle_test.go b/heicode/controller/secret_lifecycle_test.go index 8033c39..582779a 100644 --- a/heicode/controller/secret_lifecycle_test.go +++ b/heicode/controller/secret_lifecycle_test.go @@ -7,6 +7,23 @@ import ( "github.com/stretchr/testify/require" ) +// #33: an unscoped (empty-prefix) purge must be refused unless the operator +// explicitly declares the vault exclusive to HM, so HM never permanently purges +// another tenant's soft-deleted secrets in a shared vault. +func TestSecretPurgeScopeAllowed(t *testing.T) { + allowed, desc := secretPurgeScopeAllowed("", false) + require.False(t, allowed) + require.Contains(t, desc, "refusing to purge an entire") + + allowed, desc = secretPurgeScopeAllowed(" ", true) + require.True(t, allowed) + require.Contains(t, desc, "ENTIRE vault") + + allowed, desc = secretPurgeScopeAllowed("heicode-", false) + require.True(t, allowed) + require.Contains(t, desc, "heicode-") +} + func TestParseDeletedSecretsPage(t *testing.T) { body := []byte(`{ "value": [