feat(secrets): managed secrets stored locally and injected into workspaces#647
Draft
skevetter wants to merge 21 commits into
Draft
feat(secrets): managed secrets stored locally and injected into workspaces#647skevetter wants to merge 21 commits into
skevetter wants to merge 21 commits into
Conversation
✅ Deploy Preview for images-devsy-sh canceled.
|
✅ Deploy Preview for devsydev canceled.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add a devsy secret command group (set/get/list/delete/attach/detach) that stores named secrets locally in the OS keyring, with an age-encrypted file fallback whose key is auto-generated (or passphrase-derived) and recorded in a metadata index. Secrets are per-context and can be bound to a context. Inject stored secrets into workspaces with 'up --secret NAME[,type=env|mount] [,target=X]': env delivery sets a lifecycle env var; mount delivery writes the value to a tmpfs at /run/secrets. Secret values travel to the container over the agent tunnel (not argv) and are redacted from lifecycle logs. A driver mount capability lets the Kubernetes driver map tmpfs to an in-memory emptyDir and rejects unsupported providers with a clear error. --secrets-file now expects flat JSON to match the devcontainer CLI. Includes CLI, desktop UI, docs, and e2e coverage.
skevetter
force-pushed
the
feat/managed-secrets
branch
from
July 24, 2026 04:26
54327d3 to
0758ded
Compare
Wire --build-secret values into image builds as BuildKit secrets so Dockerfiles consume them via RUN --mount=type=secret,id=NAME. The internal BuildKit builder serves them through a session secret provider; the docker buildx strategy passes them as env-backed --secret references (never argv or layers). Docker compose builds, which cannot mount build secrets, reject them with a clear error.
Wire the resolved --git-token into the credential path: the tunnel GitCredentials RPC returns the token only for its own host (never leaked to other hosts git contacts), served at the up and setup clone sites. Foundation resolves the token secret and infers the username from the repo host.
Add a per-entry Sensitive flag to the managed-value store: sensitive values go to the keyring/encrypted-file backend as before; non-sensitive values are stored inline in the index (no keyring, no encryption). Add a 'devsy env' facade (set/get/list/delete) over the same store for non-sensitive vars, and an 'up --env NAME[=TARGET]' flag to inject them. 'devsy secret' and 'devsy env' are thin facades differing only in sensitivity.
UpCmd.BuildSecrets shadowed the embedded CLIOptions.BuildSecrets, so resolved build secrets were written to the flag field and never reached CLIOptions (BuildKit saw none). Rename the flag field to BuildSecretNames and write resolved values to CLIOptions. Also split resolveStoredSecrets into per-type appliers (cyclop) and add RunWithEnv for buildx build secrets.
- Strip GitToken and BuildSecrets from the CLIOptions embedded in the container setup command argv (they were recoverable via ps/proc; now delivered only over the tunnel) [security] - Enforce the sensitive/non-sensitive boundary on reads: secret get/list/delete operate on secrets only, env get/delete on env vars only [security] - Guard the index so a sensitive value can never be serialized inline - Clear the stale backend value when an entry changes from sensitive to non-sensitive - Only verify the key source when a sensitive entry is actually accessed - Add Store.Meta for kind-aware facade checks
The CLI command group was renamed secrets->secret with no alias; the desktop IPC handlers still shelled to 'secrets', breaking every Secrets-page action.
Apply idiomatic-Go and desktop-UI review findings: - Replace opaque sensitive bool in Store API with a typed Kind enum (KindSecret/KindEnv), matching the repo's Backend/DockerBuilder pattern - Drop hand-rolled splitKeyValue for strings.Cut - Register secret/env value flags via pkg/flags builder + names constants - Restructure cmd/env into one-file-per-subcommand with exported *Cmd types - Add desktop env-var UI: EnvVar type, env_* IPC handlers, invokers, store, EnvPage, /env route, sidebar nav (plaintext value UX with reveal toggle) - Fix stale 'devsy secrets list' comment in ipc.ts
…tore) - Drop secret_attach/secret_detach IPC handlers and secretAttach/secretDetach invokers: never referenced by the renderer. CLI attach/detach stays; it is a live feature (bound context.Secrets are read at up via collectSecretRequests). - Remove secrets.NewStore(): zero callers, all paths use NewStoreForConfig.
- HIGH: redact build-secret values in debug log (build.go dumped BuildOptions with %+v, exposing NAME=VALUE plaintext despite out-of-band buildx delivery) - Harden auto key generation against a first-init TOCTOU: write the key file exclusively (O_EXCL, first-wins) and re-read after save so concurrent initializers converge on one persisted key instead of undecryptable splits - Fail --git-token when the source has no HTTP(S) host rather than building an unscoped token with Host="" - Align cmd/secrets Run methods with cmd/env: unused ctx -> _ context.Context
- HIGH: reject a sensitive secret passed via --env. applyEnvVars routed the value into WorkspaceEnv, which is NOT stripped from the compressed (gzip+ base64, reversible) setup argv, exposing it in the host process list. Now errors and directs to --secret (tunnel-delivered). - HIGH: path-traversal in writeSecretFiles. A mount target like target=../../etc/x escaped /run/secrets via filepath.Join; add secretMountPath containment guard. - HIGH: unset Kind silently downgraded a secret to a plaintext env var on load (empty kind => Sensitive()==false => Get returns ""). Normalize unknown Kind to KindSecret (fail-safe). - MEDIUM: index/backend writes were non-atomic and unlocked. Add atomicWriteFile (temp+rename) so a crash cannot corrupt the store, and a cross-process flock around store set/delete/last-used so concurrent writes cannot lose entries. - MEDIUM: key-gen convergence did not hold for the keyring backend; serialize auto-key resolution with a cross-process lock (covers both backends). - LOW: mount-secret values were absent from the lifecycle-hook log redactor; add them for redaction only (never injected into hook env). Adds tests for each. 1313 tests pass; golangci-lint clean.
- MEDIUM: --git-token was silently dropped on the SSH machine-provider up path (agent.go devsyUpMachineSSH omitted WithGitToken while all other up paths wire it). Add tunnelserver.WithGitToken so the token is delivered there too. - MEDIUM: harden mount-secret writes against symlink redirection. secretMountPath now requires a plain filename (no separators/./..), and writeSecretFile does Remove-then-O_EXCL-create so a pre-planted symlink under /run/secrets cannot redirect the write+chown to an arbitrary file. Also fixes the nested-target wart (nested targets are now rejected at the guard, not silently at write). - LOW: redact mount-secret values in deferred/post-attach hook logs. Those re-exec'd processes lack the mount values in env; MountSecretsForRedaction reads them back from /run/secrets for the log redactor only (not injected). - LOW: normalizeKinds now clears any inline Value when flipping an unset Kind to secret, upholding the no-inline-plaintext invariant for hand-edited entries. - INFO: document that localStore.lock() is non-reentrant. Adds tests. Touched packages: 1082 tests pass; golangci-lint clean.
Remove comments that restated the code and tighten the rest to non-obvious rationale only, per the self-documenting-code convention. Keeps security justifications (#nosec, argv/redaction invariants, lock non-reentrancy) and exported-symbol docs.
- MEDIUM: multiline secret values (PEM keys, certs) were truncated at the first newline when passed to deferred/post-attach hooks via DEVSY_SECRETS_ENV (newline-joined). Base64-encode each entry symmetrically so any bytes survive; also stopped the dropped tail from escaping the log redactor. - LOW: reject two mount secrets sharing a target= at parse time with a clear message instead of a confusing O_EXCL 'file exists' error mid-setup. Also makes collectSecretRequests output deterministic (sorted). - Idiom: replace the confusing applier closure-slice in resolveStoredSecrets with a small secretResolver value type; append(append(...)) -> slices.Concat; extract the duplicated flock logic into a shared acquireFlock helper. Adds tests. 1428 tests pass; golangci-lint clean.
runRawStdin passed { input } to Node's promisified execFile, but that option is
ignored by the async execFile (only execFileSync honors it). The CLI's
'secret set --stdin' then blocked forever on io.ReadAll(os.Stdin) with no value
and no EOF, so the desktop 'Add Secret' spinner never finished; because the call
holds a concurrency slot, retries hung too. Spawn the child and write+close
stdin explicitly instead.
Mirrors the existing --secret injection e2e for feature parity: env set a var, devsy up --env NAME, assert it lands in the workspace login-shell environment. Adds a storeEnv helper alongside storeSecret.
Regression coverage for the desktop 'Saving...' hang:
- cmd/secrets: resolveValue reads --stdin (incl. multiline PEM preservation
and the trailing-newline trim), --value, and rejects multiple sources.
- desktop cli.test.ts: runRawStdin writes the value to the child's stdin and
resolves; rejects with stderr on non-zero exit. Guards against the
execFile { input } no-op that left the CLI blocked reading stdin.
The Add dialog silently upserted, so re-using a name replaced a value with no signal — irreversible for secrets (write-only, no version history). Detect the name collision against the loaded list and make the overwrite explicit: - Secrets: title -> 'Replace Secret', amber 'will replace, cannot be undone' warning, destructive-styled 'Replace' button. - Env vars (lower stakes, values visible): title -> 'Update', muted 'will update its value' note, 'Update' button. Store semantics unchanged (upsert stays; rotation via CLI still works).
…rumbs The Cmd+K palette had no Secrets or Env Vars entries, and the breadcrumb nav had no labels for /env or /ssh-keys. Add: - Palette Navigation entries for Secrets and Env Vars pages. - Palette dynamic entries for each stored secret/env var (searchable by name, navigates to the page), mirroring workspaces/providers/machines. - Breadcrumb labels for secrets, env, and ssh-keys.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a persistent, managed secrets feature. Previously secrets were ephemeral (a
--secrets-fileparsed atuptime); users can now store named secrets once and reference them by name.pkg/secrets): aStorebehind a smallbackendinterface with two implementations — the OS keyring (zalando/go-keyring) and anage-encrypted file fallback (filippo.io/age) for headless/CI. Only non-sensitive metadata (names, timestamps) is written to~/.devsy; values live in the backend, which is the source of truth. A YAML index enables listing since keyrings can't enumerate.SECRETS_BACKENDcontext option (auto/keyring/file), overridable per-invocation viaDEVSY_SECRETS_BACKEND.devsy secrets set/list/get/delete/attach/detach. Values are read via stdin/hidden prompt to keep them off argv.devsy up --secret NAME[=ENV_VAR](repeatable) plus per-context binding viaattach. Resolves into the existing lifecycle-env path. A missing requested/bound secret is a hard error, not a silent drop.SecretsPage+ nav entry.Secrets are per-context; deleting a context cascades to its secrets.
Notes / follow-ups
filebackend requiresDEVSY_SECRETS_PASSPHRASE; without a passphrase and without a usable keyring, operations fail with a clear error rather than proceeding insecurely.proserver-side secret sync is out of scope for this PR.New dependencies (
zalando/go-keyring,filippo.io/age) are cgo-free and cross-compile clean for all release targets.