From 8fe1f5e131b125ec0fb254fd69514d3a3ac6c085 Mon Sep 17 00:00:00 2001 From: chenchen Date: Wed, 10 Jun 2026 00:33:47 +0800 Subject: [PATCH 1/2] 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 03cdb1dd..aa13b5d5 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 837717e9..7f622c05 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 0399b261..406945af 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 8033c39a..582779a3 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": [ From 2e37495133f2dc388d7736b1ceb7094931752312 Mon Sep 17 00:00:00 2001 From: chenchen Date: Wed, 10 Jun 2026 00:40:23 +0800 Subject: [PATCH 2/2] feat(telemetry): retention purge + context field whitelist + size caps (#32) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Telemetry up-gating hardening (code portion of #32): - Context field whitelist: telemetry `context` is filtered to a small set of non-content diagnostic keys (route/retryable/phase/exit_code/duration_ms/ attempt) before persistence. Unknown keys — including potentially identifying ones (email, full file path, prompt, raw IP) — are dropped, so a client regression cannot land arbitrary JSON in the store. Empty/unparseable/no-allowed-key context is dropped to "". - Per-field size cap: stack_top and context are truncated to 8KiB after redaction (backstop against unbounded blobs within batch limits). - Retention: daily master-only task deletes telemetry rows older than HEICODE_TELEMETRY_RETENTION_DAYS (default 30; <=0 disables). HEICODE_TELEMETRY_RETENTION_INTERVAL_HOURS (default 24) sets cadence. model.DeleteTelemetryEventsBefore(cutoff) + controller.StartTelemetryRetentionTask() wired into main.go under IsMasterNode. - GET /api/heicode/config telemetry block now surfaces retention_days for client/admin transparency. Tests: whitelist drop/keep, size cap, redaction-within-allowed-key. go build/vet clean; controller telemetry tests pass. Affects: Manager only (telemetry ingest + retention). No billing/consume-log change (telemetry still never bills). Privacy-doc disclosure + production enable-checklist portions of #32 tracked in heicodeDocs sync (#34) / desktop client API docs (#35). Co-Authored-By: Claude Opus 4.8 --- heicode/controller/heicode_config.go | 2 + heicode/controller/heicode_telemetry.go | 66 +++++++++++++++-- heicode/controller/heicode_telemetry_test.go | 37 +++++++++- heicode/controller/telemetry_retention.go | 77 ++++++++++++++++++++ heicode/main.go | 3 + heicode/model/telemetry_event.go | 12 +++ 6 files changed, 190 insertions(+), 7 deletions(-) create mode 100644 heicode/controller/telemetry_retention.go diff --git a/heicode/controller/heicode_config.go b/heicode/controller/heicode_config.go index 30329bcc..367851f6 100644 --- a/heicode/controller/heicode_config.go +++ b/heicode/controller/heicode_config.go @@ -24,6 +24,8 @@ func HeicodeConfig(c *gin.Context) { "endpoint": "/api/heicode/telemetry/events", "max_batch": common.GetEnvOrDefault("HEICODE_TELEMETRY_MAX_BATCH", telemetryMaxBatch), "flush_interval_sec": common.GetEnvOrDefault("HEICODE_TELEMETRY_FLUSH_INTERVAL_SEC", 30), + // Server retention window (#32): events older than this are purged. + "retention_days": telemetryRetentionDays(), }, }) } diff --git a/heicode/controller/heicode_telemetry.go b/heicode/controller/heicode_telemetry.go index 00956665..a64b0736 100644 --- a/heicode/controller/heicode_telemetry.go +++ b/heicode/controller/heicode_telemetry.go @@ -25,8 +25,65 @@ const ( telemetryMaxBatch = 20 telemetryMaxBodySize = 256 * 1024 headerDeviceID = "X-Heicode-Device-Id" + // Per-field hard cap after redaction (#32): a backstop so a single event + // can't park an unbounded blob in the telemetry store even within batch + // limits. stack_top / context are truncated past this many bytes. + telemetryMaxFieldBytes = 8 * 1024 ) +// telemetryContextAllowedKeys whitelists the non-content diagnostic keys the +// client may attach to an event's `context` (#32). Anything else is dropped +// before persistence, so a client regression can't land arbitrary — possibly +// identifying — JSON (prompts, code, tokens, emails, full file paths, raw IPs) +// in the telemetry store. Keep in sync with the client telemetry contract +// (winos#23) and docs/integration/heicode-desktop-client-api.md. Additions must +// be reviewed against the "no identifying content" rule in issue #32. +var telemetryContextAllowedKeys = map[string]bool{ + "route": true, // logical UI route, e.g. "chat" (no params) + "retryable": true, // bool + "phase": true, // lifecycle phase enum + "exit_code": true, // process exit code (int) + "duration_ms": true, // numeric timing + "attempt": true, // retry attempt count +} + +// filterTelemetryContext keeps only whitelisted keys from the client-supplied +// context object, then redacts and size-caps the result (#32). Returns "" when +// the context is empty, unparseable, or has no allowed keys — telemetry is +// best-effort diagnostics, so dropping an unrecognized payload is preferable to +// storing arbitrary JSON. +func filterTelemetryContext(raw json.RawMessage) string { + if len(raw) == 0 { + return "" + } + var obj map[string]json.RawMessage + if err := common.Unmarshal(raw, &obj); err != nil { + return "" // not an object (or malformed) -> drop + } + filtered := make(map[string]json.RawMessage, len(obj)) + for k, v := range obj { + if telemetryContextAllowedKeys[k] { + filtered[k] = v + } + } + if len(filtered) == 0 { + return "" + } + b, err := common.Marshal(filtered) + if err != nil { + return "" + } + return capTelemetryField(model.RedactText(string(b))) +} + +// capTelemetryField truncates an already-redacted field to telemetryMaxFieldBytes. +func capTelemetryField(s string) string { + if len(s) <= telemetryMaxFieldBytes { + return s + } + return s[:telemetryMaxFieldBytes] +} + type telemetryEventIn struct { ClientId string `json:"client_id"` SchemaVersion int `json:"schema_version"` @@ -73,13 +130,12 @@ func (e telemetryEventIn) toModel(userID int, deviceID string, now int64) model. stackTopJSON := "" if len(e.StackTop) > 0 { if b, err := common.Marshal(e.StackTop); err == nil { - stackTopJSON = model.RedactText(string(b)) + stackTopJSON = capTelemetryField(model.RedactText(string(b))) } } - ctxJSON := "" - if len(e.Context) > 0 { - ctxJSON = model.RedactText(string(e.Context)) - } + // #32: context is restricted to a key whitelist (drop arbitrary/identifying + // JSON), then redacted and size-capped. + ctxJSON := filterTelemetryContext(e.Context) return model.TelemetryEvent{ ReceivedAt: now, UserId: userID, diff --git a/heicode/controller/heicode_telemetry_test.go b/heicode/controller/heicode_telemetry_test.go index b9dec944..aa9f0535 100644 --- a/heicode/controller/heicode_telemetry_test.go +++ b/heicode/controller/heicode_telemetry_test.go @@ -65,12 +65,45 @@ func TestTelemetryToModel_RedactsSecrets(t *testing.T) { ev := telemetryEventIn{ ClientId: "dev-1", StackTop: []string{"at boom (auth.ts) key=sk-abcDEF1234567890"}, - Context: json.RawMessage(`{"hdr":"Authorization: Bearer aZ09tokenVALUE","ok":true}`), + // "route" is whitelisted (#32) so it survives the field filter; redaction + // must still strip the Bearer token carried inside an allowed key. + Context: json.RawMessage(`{"route":"Authorization: Bearer aZ09tokenVALUE","retryable":true}`), } m := ev.toModel(7, "dev-1", 1700000000) require.NotContains(t, m.StackTopJSON, "sk-abcDEF1234567890", "sk- secret must be redacted in stack_top") require.Contains(t, m.StackTopJSON, "REDACTED") require.NotContains(t, m.ContextJSON, "aZ09tokenVALUE", "Bearer token must be redacted in context") - require.Contains(t, m.ContextJSON, "ok") // non-secret content preserved + require.Contains(t, m.ContextJSON, "retryable") // non-secret whitelisted content preserved +} + +// #32: context must be restricted to a key whitelist so a client regression +// cannot land arbitrary/identifying JSON in the telemetry store. +func TestFilterTelemetryContext_Whitelist(t *testing.T) { + // allowed keys kept, unknown keys (incl. potentially identifying) dropped + out := filterTelemetryContext(json.RawMessage( + `{"route":"chat","retryable":true,"email":"a@b.com","file":"C:/Users/x/secret.go","prompt":"hi"}`)) + require.Contains(t, out, "route") + require.Contains(t, out, "retryable") + require.NotContains(t, out, "email") + require.NotContains(t, out, "a@b.com") + require.NotContains(t, out, "secret.go") + require.NotContains(t, out, "prompt") + + // no allowed keys -> dropped entirely + require.Equal(t, "", filterTelemetryContext(json.RawMessage(`{"email":"a@b.com"}`))) + // non-object / malformed -> dropped + require.Equal(t, "", filterTelemetryContext(json.RawMessage(`"a string"`))) + require.Equal(t, "", filterTelemetryContext(json.RawMessage(`not json`))) + require.Equal(t, "", filterTelemetryContext(nil)) +} + +// #32: per-field size cap is a backstop against unbounded blobs. +func TestCapTelemetryField(t *testing.T) { + require.Equal(t, "short", capTelemetryField("short")) + big := make([]byte, telemetryMaxFieldBytes+100) + for i := range big { + big[i] = 'a' + } + require.Len(t, capTelemetryField(string(big)), telemetryMaxFieldBytes) } diff --git a/heicode/controller/telemetry_retention.go b/heicode/controller/telemetry_retention.go new file mode 100644 index 00000000..5601b1ab --- /dev/null +++ b/heicode/controller/telemetry_retention.go @@ -0,0 +1,77 @@ +package controller + +import ( + "fmt" + "sync" + "time" + + "github.com/heicode/manager/common" + "github.com/heicode/manager/model" +) + +// Telemetry retention (#32). Account-linkable client error-telemetry must not be +// kept indefinitely: a daily task deletes rows older than the retention window. +// Disabled when telemetry ingest is off (default) or retention <= 0. +// +// - HEICODE_TELEMETRY_RETENTION_DAYS (default 30): rows received earlier than +// now-RETENTION are deleted. <=0 disables the purge (keep-forever — only for +// explicit operator opt-out; not recommended for production). +// - HEICODE_TELEMETRY_RETENTION_INTERVAL_HOURS (default 24): sweep cadence. +// +// Master-only (wired from main.go under IsMasterNode) so multiple nodes don't +// all sweep the shared LOG_DB. + +var telemetryRetentionTaskOnce sync.Once + +func telemetryRetentionDays() int { + return common.GetEnvOrDefault("HEICODE_TELEMETRY_RETENTION_DAYS", 30) +} + +// StartTelemetryRetentionTask launches the daily telemetry retention sweep. +func StartTelemetryRetentionTask() { + telemetryRetentionTaskOnce.Do(func() { + // Only meaningful once ingest is enabled; if the endpoint is off there is + // nothing being written, but we still allow the sweep to drain any rows + // captured during a prior enabled window. Gate on retention days instead. + days := telemetryRetentionDays() + if days <= 0 { + common.SysLog("telemetry retention task disabled (HEICODE_TELEMETRY_RETENTION_DAYS<=0; rows kept indefinitely)") + return + } + intervalHours := common.GetEnvOrDefault("HEICODE_TELEMETRY_RETENTION_INTERVAL_HOURS", 24) + if intervalHours < 1 { + intervalHours = 24 + } + go func() { + time.Sleep(5 * time.Minute) // avoid startup churn + runTelemetryRetentionOnce() + ticker := time.NewTicker(time.Duration(intervalHours) * time.Hour) + defer ticker.Stop() + for range ticker.C { + runTelemetryRetentionOnce() + } + }() + common.SysLog(fmt.Sprintf("telemetry retention task started: retention=%dd interval=%dh", days, intervalHours)) + }) +} + +func runTelemetryRetentionOnce() { + defer func() { + if r := recover(); r != nil { + common.SysLog(fmt.Sprintf("telemetry retention task panic recovered: %v", r)) + } + }() + days := telemetryRetentionDays() + if days <= 0 { + return + } + cutoff := time.Now().Unix() - int64(days)*86400 + deleted, err := model.DeleteTelemetryEventsBefore(cutoff) + if err != nil { + common.SysLog("telemetry retention task: " + err.Error()) + return + } + if deleted > 0 { + common.SysLog(fmt.Sprintf("telemetry retention task: deleted %d events older than %d days", deleted, days)) + } +} diff --git a/heicode/main.go b/heicode/main.go index 77812f70..3b04573f 100644 --- a/heicode/main.go +++ b/heicode/main.go @@ -134,6 +134,9 @@ func main() { // retention window (issue #4). Master-only so multiple nodes don't all purge. if common.IsMasterNode { controller.StartSecretPurgeTask() + // Telemetry retention: daily purge of client error-telemetry older than + // HEICODE_TELEMETRY_RETENTION_DAYS (#32). Master-only. + controller.StartTelemetryRetentionTask() } if common.IsMasterNode && constant.UpdateTask { diff --git a/heicode/model/telemetry_event.go b/heicode/model/telemetry_event.go index c7e23c13..65fac5ce 100644 --- a/heicode/model/telemetry_event.go +++ b/heicode/model/telemetry_event.go @@ -39,3 +39,15 @@ func InsertTelemetryEvents(events []TelemetryEvent) error { } return LOG_DB.Create(&events).Error } + +// DeleteTelemetryEventsBefore removes telemetry rows received before cutoffUnix +// (server unix seconds), enforcing the retention window (#32). Returns the +// number of rows deleted. Account-linkable device telemetry must not be kept +// indefinitely; callers run this from a periodic retention task. +func DeleteTelemetryEventsBefore(cutoffUnix int64) (int64, error) { + if LOG_DB == nil { + return 0, nil + } + res := LOG_DB.Where("received_at < ?", cutoffUnix).Delete(&TelemetryEvent{}) + return res.RowsAffected, res.Error +}