Repository navigation
Replies: 5 comments 3 replies
|
Thanks for the detailed RFC, @amasolov, and for grounding it in the code. We'll take it forward here as a discussion. We checked the motivation and it holds: credentials only come in through the API,
On audit, we don't see a need for extra work. Heym's audit lines come from the API on the main instance, and these credentials are changed outside Heym, so the record of who changed what already lives where the Secret is managed: git history for GitOps, the secret store's own audit log, and the Kubernetes audit log when it's enabled. Teams usually stream to Splunk, VictoriaLogs or similar.
Separately, we think rotation support for |
|
the memory-only, never-written-to-db constraint is the right call. one thing to double-check: the ORM set event write-back guard is good, but bulk updates or raw SQL that bypass the ORM (migrations, admin scripts) could still write stale data back silently. worth adding an integration test that intentionally tries a raw update and verifies it fails or no-ops. the re-queue approach for startup is cleaner than blocking, but document that the window can be nonzero even after the pod is "ready" so operators don't wire up health checks that miss it. separately, since heym is an agent runner, it's worth thinking about whether the credential values themselves can end up in model context (prompt, tool output, logs) during a run, which is a different surface than the db. that's the problem we built 1Claw to address, path references to vault tokens that never surface the raw value. full disclosure, i work on 1Claw. not saying you need it, just flagging it as a layer this RFC doesn't cover. |
|
I've updated the RFC to revision 3. The changes are listed at the top; here's the reasoning. RabbitMQ revision. @ckakgun, your point that a revision would also fix UI edits holds: Delayed re-queue. Agreed. @redbotster: raw SQL and migrations. Good catch. An ORM event can't see them. The migration that adds Still open (§ Open questions): the owner model, teams by name, the annotation prefix, and the grace period on delete. Your call on those would let me start on the core PR. |
|
Thanks for revision 3, @amasolov, and thanks @redbotster for the review. Check constraint: as written, it refuses the watcher's own rows. Re-queue before the first sync: Config hash: sounds good. Since the key would hold a hash of a secret, an HMAC with a per-process random key over canonical JSON ( Open questions:
On values reaching a model, we agree it's outside this RFC, since it doesn't change how a run uses a credential. We're happy to take all of it in a single PR, the RabbitMQ fix included. Thanks again 🙏 |
|
The implementation is up as #867: one PR, with the RabbitMQ fix as its first commit. Its description covers the live tests on a kind cluster and the three places it departs from revision 4: how a run fails before the first sync, making room under the executor's line limit, and when the write guard checks a pointer without a source. |

Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Changes in revision 4
Following @ckakgun's review of revision 3 (reply):
sourceholds the full pointer, and the check isencrypted_config = source. Revision 3's check comparedk8s:ns/namewithk8s:k8s:ns/nameand refused the watcher's own rows (§3).not_beforecolumn and the second migration are gone (§4).mainthat stopped any reconnect, and they're fixed in the same commit (§4).Changes in revision 3
Following @ckakgun's reply, @redbotster's comment and the #862 review:
resourceVersionorupdated_at. It changes only when the value does, so a UI edit reconnects, and a key-rotation rewrite doesn't (§4). Revision 4: an HMAC, and it ships in the single PR.not_beforecolumn onworkflow_run_queue. Superseded in revision 4: unsynced processes don't claim runs.CHECK, which also covers raw SQL, migrations and admin scripts (§3, §7). Corrected in revision 4.Changes in revision 2
Following @ckakgun's review:
encrypt_config(), which can't see the row, to an ORMsetevent onCredential.encrypted_config(§7).subprocesssandbox modes are refused while the feature is on (§2, Security).ENCRYPTION_KEYrotation moved to its own RFC, RFC: Online rotation for ENCRYPTION_KEY (MultiFernet) #862.Summary
On Kubernetes, let a labelled
Secretbe a Heym credential. Heym watches Secrets in its namespace that carryheym.run/credential: "true". Each one appears as a read-only credential: annotations give its name, type, owner and sharing, and the Secret's data is its config. Values live in memory only and are never written to Heym's database. Updating the Secret rotates the credential immediately.Heym only needs Kubernetes, not a particular secret store. Anything that writes a Secret works: External Secrets Operator, SealedSecrets, SOPS through Flux or Argo CD, or
kubectl. That lets operators keep credentials in Vault, AWS Parameter Store, 1Password or similar, and declare them in git next to the rest of the deployment.Motivation
Today a credential can only be created by a person in the UI (
api/credentials.py:1223). Its config is encrypted withENCRYPTION_KEYand stored incredentials.encrypted_config(db/models.py:1110). For teams that run Heym on Kubernetes from git:ENCRYPTION_KEY, which can't be rotated. Rotation itself is RFC: Online rotation for ENCRYPTION_KEY (MultiFernet) #862.Heym already treats Kubernetes as a first-class platform. It has an in-cluster API client (
services/sandbox/kubernetes/client.py), runs sandbox pods through it, and ships a Kubernetes deployment guide. This feature is a small module next to that code.Goals
$credentials, sharing and output masking.Non-goals (v1)
ExternalSecretobjects. Heym reads Secrets and nothing else; how a Secret gets made is the operator's choice.codex,google_sheets,google_drive,bigquery, and the OAuth modes oflinearandnotion). Heym writes refreshed tokens back to these, so they can't be read-only.Design
1. The Secret
Config comes from the Secret's data, in one of two forms:
port: "5432"orsecure: "true"are converted by the credential type's existing validation (validate_credential_config).config.jsonkey, for types with nested config. It's parsed as the whole config.Non-secret fields (
base_url,host) can sit in the Secret too. Keeping the whole credential in one object is the point.With External Secrets Operator, the
ExternalSecret's target template carries the label and annotations:2. Enabling it
HEYM_K8S_CREDENTIALSfalseHEYM_K8S_CREDENTIALS_NAMESPACEStartup refuses to run with the feature on in two cases. Both are configuration errors that won't fix themselves:
KubeClient.can_i(client.py:122). If the API server answers thatlistorwatchonsecretsis denied, startup fails with a message naming the missing permission. If the API server can't be reached, that's not a configuration error: startup continues, and the check runs again with the first sync (§4).subprocesssandbox mode.HEYM_PYTHON_TOOL_SANDBOX(also used by skills),HEYM_MCP_STDIO_SANDBOXandHEYM_PLAYWRIGHT_SANDBOXeach acceptsubprocess, which runs user code inside the backend container, next to the ServiceAccount token. Scrubbing the environment, as those paths do, doesn't help, because the token is a file. Startup names whichever switch is set.deploy/kubernetesgains a Role grantingget,listandwatchonsecretsin that namespace, and the variables. The community Helm chart follows withkubernetesCredentials.enabledand.namespace.3. Storage: a row without a value
Credentials still need a database row: nodes reference credentials by ID, and sharing is stored as rows. For a Secret-backed credential:
idis deterministic:uuid5(HEYM_NS, "<namespace>/<secret-name>"). If the Secret is deleted and recreated (a GitOps prune and re-apply, or ESO recreating its target), the credential keeps its ID, so workflows that reference it keep working.encrypted_configholds a pointer, not a ciphertext:k8s:<namespace>/<secret-name>. It contains nothing secret.source, holds the same full pointer (k8s:<namespace>/<secret-name>). The API and UI use it to decide that a row is read-only. The migration adds it, together with a check constraint:CHECK (source IS NULL OR encrypted_config = source). Postgres then refuses a real value on a Secret-backed row, whatever writes it (§7). Keeping the prefix insourcelets a later file source (file:…) use the same constraint unchanged. The constraint is tested against a real Postgres, so a mismatch like revision 3's can't pass unnoticed.4. Resolution
Every process (each API worker and each cluster worker) runs a watch on
secrets?labelSelector=heym.run/credential=trueand keeps decoded configs in memory. The cache behaves like a client-go informer:--workers 8), so blocking each one on the API server would keep the UI down whenever the API server is unreachable.get_credentials_contextdrops credentials it can't resolve, so a run only finds out at the node. With clustering off, runs execute in-process and never reach the queue anyway. Instead:claim_next.credentials_synced_at = now()on each heartbeat. Sync state belongs to each process, butcluster_instanceshas one row per instance that all its processes upsert, so the upsert keeps the newer value (GREATEST), and an unsynced sibling can't clear it.heartbeat_at, and skips it otherwise, as it does a mismatched instance. Queued rows then go to an instance that has synced.credential "x" is not synced from Kubernetes yet. Until the first sync, Secret-backed credentials enter the credentials context as a marker that raises that error, rather than being missing.Every read of a credential goes through
decrypt_config(): 111 calls in 64 files. Resolution goes inside it, so none of those call sites change:Rotation. A watch event replaces the cached config, and the next use gets the new value. Most readers already pick it up:
$credentialsredis_pool.py:15), so a rotated secret gets a new pool. Nothing to change.rabbitmq_consumer.py:195). The consumer key becomes{workflow_id}_{node_id}_{credential_id}_{queue_name}_{revision}, where the revision is a short HMAC of the decrypted config as canonical JSON, under a random key generated per process, for every credential. An HMAC keeps a weak password from being checkable if the key ever reaches a log, and only the leader holds consumers, so revisions never need to match across processes._sync_consumersstops consumers whose key disappears before starting new ones, so the leader reconnects with the new value on its next 30-second sync. The revision changes only when the value does: not on an annotation edit, and not when #862's job re-encrypts every row. It also fixes today's gap where editing a RabbitMQ credential, switching to another one or renaming the queue keeps the old connection. Two bugs onmainare fixed with it, because they stopped any reconnect from working:_stop_consumercalledchannel.cancel(), which aio-pika channels don't have, so a removed or deactivated trigger kept consuming (and a deactivated one redelivered each message in a tight loop); and the start task was created without a reference, so it could be garbage-collected mid-connect.websocketHeadersrather than from credentials, and the credentials context is built per event (websocket_trigger_service.py:419).5. Syncing rows
Rows (not values) are created, updated and soft-deleted from watch events by one process at a time, under
DistributedLockService:credential "x": Secret ns/name not found. Rows are never deleted automatically: an admin removes them from the admin view (§6). If the Secret comes back, the row becomes available again with the same ID.6. Read-only in the API and UI
For rows with
sourceset:PUT,DELETEand the share endpoints return 409"Managed by Kubernetes Secret ns/name".k8s:ns/name), notmask_api_key()of the real value, which reveals its first 7 characters (api/credentials.py:398).7. Write-back guard
OAuth token refresh,
codex_nodeand thePUThandler decrypt a credential, change it and encrypt it again. On a Secret-backed row, that would write the real value into the database.heym.run/type.encrypt_config()only sees the config, not the row, so it can't tell. Instead, asetevent onCredential.encrypted_configraises for any value other than thek8s:pointer when the row'ssourceis set. It follows thelistens_forpattern already inmodels.py, and a later write-back site fails loudly in tests instead of leaking quietly.encrypted_configare ORM attribute assignments orCredential(...)constructors, and attribute events fire for constructor arguments too. There are no Core-levelupdate()/insert()statements oncredentialsand no raw SQL.sourceset. The ORM event stays, because it fails earlier, with a message that names the credential. A test runs a rawUPDATEagainst a Secret-backed row and expects the constraint to refuse it.Security
k8s:rows. The API always storesencrypt(config), whose Fernet token starts withgAAAAA. No request body can produce a row that resolves from a Secret, and there's no reference syntax a user could aim at files or environment variables.listandwatchonsecretscan't be narrowed by label, so the backend's token can read every Secret in the watched namespace. Heym's own keys are in its environment anyway, but other apps' Secrets shouldn't share that namespace. The guide should say: run Heym in its own namespace, or pointHEYM_K8S_CREDENTIALS_NAMESPACEat a namespace that holds only Heym credentials.automountServiceAccountToken: false(sandbox/kubernetes/pod.py:207), and startup refuses thesubprocesssandbox modes (§2).ENCRYPTION_KEYdoesn't affect these credentials.mask_sensitive_outputmasks every value in the credentials context, whatever its source. Secret-backed values reach a run, and from there a prompt or a tool, exactly as UI credentials do today; this RFC changes where a value is stored, not how a run uses it.Alternatives considered
$__file{}references (Grafana-style provisioning)$__file{/proc/self/environ}, a mount on every workload (with asubPathtrap that turns rotation off), and a polling reconciler. Worth doing later as a second source, but it shouldn't block the Kubernetes case.ExternalSecretobjects directlyFuture work
sourcecolumn, read-only path and write-back guard. Only the loader is new.heym.run/kind: sso). That would make the security-critical toggles (account creation on first sign-in, password sign-in) declarative.kubectl describeshows them. This needscreateonevents.Decided
heym.run/, which the sandbox pods already use.Delivery
One PR, merged with its commits, and one migration: the
sourcecolumn and its check constraint, andcredentials_synced_atoncluster_instances. Commits, in order:sourcecolumn and its check constraint, thek8s:pointer indecrypt_config, deterministic IDs, read-only API and UI, and the write-back guard (the model event, plus tests for it and for the constraint against a real Postgres).subprocessmodes), and the admin status view.deploy/kubernetes, plus guides for ESO, plain Secrets and SOPS. The community Helm chart follows separately.All reactions