feat: OpenBao-backed secrets, vps target support - #7
Conversation
…ation before touching prod)
🤖 Augment PR SummarySummary: This PR adds OpenBao-backed secret resolution and a production VPS deployment target to the CLI. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
| // 0o600, not the 0o644 state.go uses for state.json: this file holds | ||
| // SecretID, which state.json never holds anything equivalent to. | ||
| if err := os.WriteFile(path, b, 0o600); err != nil { |
There was a problem hiding this comment.
internal/openbao/credentials.go:112: target is accepted verbatim by secrets login and reaches filepath.Join; for example, versola secrets login ../active/state ... writes ~/.versola/active/state.json instead of a credential file. A successful login can therefore corrupt deployment state (and similarly address other files beneath ~/.versola) even though Configure supports only local and vps.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // of it, and up.go's own docker volume create call is idempotent, so | ||
| // doing it again there once Up runs doesn't conflict with this one. | ||
| fragmentPath := filepath.Join(dir, "compose.fragment.yml") | ||
| if err := docker.Run("volume", "create", "versola-openbao-file"); err != nil { |
There was a problem hiding this comment.
internal/deploy/configure.go:96: This external volume is not removed by the existing docker compose down --volumes uninstall path, because Compose never removes external: true volumes. Consequently versola uninstall leaves the OpenBao data and all resolved secrets on disk, and a later fresh install silently reuses them.
Severity: medium
Other Locations
internal/deploy/up.go:77
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…me on local uninstall
|
augment review |
| // same way ComposeFile already treats them internally: no state means | ||
| // nothing to know a target for, not a reason to fail uninstall itself. | ||
| var target string | ||
| if st, loadErr := state.Load(); loadErr == nil { |
There was a problem hiding this comment.
internal/cmd/uninstall.go:54: Although this determines the active target, the later removal of ~/.versola also removes ~/.versola/openbao/*.json; uninstalling a local stack therefore deletes the cached VPS AppRole credentials (and vice versa). This defeats the new per-target credential persistence and can leave an otherwise intact target unable to configure until its credentials are reissued.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
… OpenBao credentials)
|
augment review |
| } | ||
|
|
||
| func runSecretsLogin(cmd *cobra.Command, args []string) error { | ||
| target, address, roleID, secretID := args[0], args[1], args[2], args[3] |
There was a problem hiding this comment.
internal/cmd/secrets.go:43: secretID is an AppRole password, but accepting it as a positional argument exposes it in the invoking shell's history and, while this command is running, to users able to inspect process arguments. This undermines the package's stated handling of SecretID as a value that must not be exposed.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| if owner, used := dockerPortInUse(port); used { | ||
| if owner == ownContainer { | ||
| return Result{Name: name, OK: true} |
There was a problem hiding this comment.
internal/checks/checks.go:112: Returning success here skips the raw host bind check. On the Docker Desktop/WSL2 case described above, another host process can bind this port while the current nginx container is published; replacing or restarting nginx then needs Docker to publish the port again and can fail despite this prerequisite passing.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…rtFree does a raw bind check even for our own container
…ment state across a failed redeploy; don't drop unknown OpenBao keys on write; prompt for secret-id instead of passing it as an arg; raw bind check for our own container's port
|
augment review |
| // on it — a real conflict, not a false alarm from our own container. | ||
| ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) | ||
| if err != nil { | ||
| if used { |
There was a problem hiding this comment.
internal/checks/checks.go:139: On native Docker, the deployment's own published port normally makes this net.Listen fail, so this branch rejects every local redeploy while versola-nginx is running despite the earlier own-container exception. The WSL2 behavior described in the comment does not make a bind failure distinguish an external owner on Linux/macOS.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| fmt.Println("OpenBao is already running.") | ||
| } else { | ||
| fmt.Println("Starting OpenBao...") | ||
| if err := docker.Run("compose", "-f", fragmentPath, "up", "-d", "openbao"); err != nil { |
There was a problem hiding this comment.
internal/deploy/configure.go:124: If a first configuration later fails (for example, missing credentials or a sealed OpenBao), this starts OpenBao without recording deployment state; uninstall then sees deployed == false and never runs compose down, orphaning the container and its volume after it deletes the generated bundle.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| creds, err := openbao.LoadCredentials(target) | ||
| if err != nil { | ||
| if errors.Is(err, openbao.ErrNoCredentials) { | ||
| return fmt.Errorf("no OpenBao credentials stored for %q — run `versola secrets login %s <address> <role-id> <secret-id>` first", target, target) |
There was a problem hiding this comment.
internal/cmd/secrets.go:97: This recovery command still shows a fourth positional secret-id, although login now accepts only three arguments and prompts for it; copying the guidance both fails and exposes the Secret ID in shell history/process arguments.
Severity: medium
Other Locations
internal/deploy/secrets.go:36
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
… orphaned OpenBao container on uninstall; fix stale secrets-login usage hint
|
augment review |
| target, address, roleID := args[0], args[1], args[2] | ||
|
|
||
| fmt.Print("Secret ID: ") | ||
| secretID := readLine() |
There was a problem hiding this comment.
internal/cmd/secrets.go:50: readLine leaves terminal echo enabled, so the AppRole SecretID is displayed in the terminal (and commonly retained in scrollback or session recordings). This still exposes the password even though moving it out of positional arguments avoids shell history and process-list exposure.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…session recordings)
|
augment review |
| fmt.Println("OpenBao is already running.") | ||
| } else { | ||
| fmt.Println("Starting OpenBao...") | ||
| if err := docker.Run("compose", "-f", fragmentPath, "up", "-d", "openbao"); err != nil { |
There was a problem hiding this comment.
internal/deploy/configure.go:124: down removes versola-openbao through the current Compose file; the next bootstrap reaches this line and starts a fresh container, which the code below notes is sealed on every fresh start. Because resolveSecrets immediately authenticates and there is no CLI unseal path, the documented down→bootstrap reuse flow (and a Docker daemon restart) fails until an operator manually unseals OpenBao.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Intentional — see develop.md's OpenBao one-time-setup section, single-operator unseal is by design for this internal tool
| // entries expect these files to already exist) -- Configure is where | ||
| // both of those are true. | ||
| func resolveSecrets(dir, target string) error { | ||
| creds, err := openbao.LoadCredentials(target) |
There was a problem hiding this comment.
internal/deploy/secrets.go:33: On a clean machine, Configure starts a brand-new uninitialized local OpenBao volume before this call, so no local AppRole credential can exist. secrets login local cannot prepare it beforehand because its login requires that still-uninitialized server, making the advertised fresh bootstrap local workflow fail unless OpenBao is manually initialized and configured outside the CLI.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Intentional — see develop.md's OpenBao one-time-setup section, single-operator unseal is by design for this internal tool
|
augment review |
| } | ||
| if err := os.MkdirAll(bundleDir, 0o755); err != nil { | ||
| return "", fmt.Errorf("couldn't create %s: %w", bundleDir, err) | ||
| if err := s.Save(); err != nil { |
There was a problem hiding this comment.
internal/state/state.go:206: This replacement is not failure-safe: Save uses os.WriteFile, which truncates an existing state.json before a later write failure. For example, a full disk makes Configure return an error but destroys the previous active-deployment record that this new sequencing is intended to preserve, leaving its running containers without a usable down/uninstall handle.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
| } | ||
|
|
||
| return writeDotenv(filepath.Join(dir, service+".secrets.env"), final) |
There was a problem hiding this comment.
internal/deploy/secrets.go:101: Only the resolved file is written with mode 0600; the *.generated-secrets.env source read above is left in the 0755 bundle directory. Those candidate values are secret material (and become live values on first setup), so the tools' normal readable output exposes them to other local users despite the protection applied to *.secrets.env.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…nerated-secrets.env candidates after resolving (world-readable, held real secret material)
|
augment review |
| // develop.md's OpenBao section for the manual `bao operator unseal` | ||
| // step. | ||
| fmt.Println("Resolving secrets (OpenBao)...") | ||
| if err := resolveSecrets(dir, target); err != nil { |
There was a problem hiding this comment.
internal/deploy/configure.go:149: If resolution fails (for example, because credentials are missing or OpenBao is sealed), the generated *.generated-secrets.env files remain in the 0755 bundle directory because removal only occurs after a successful per-service resolution. Those candidates become live values on first setup, so this leaves secret material readable to other local users on a shared VPS host.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
… after successful resolve
|
augment review |
| func restrictGeneratedSecretsPerms(dir string) error { | ||
| for _, service := range secretServices { | ||
| path := filepath.Join(dir, service+".generated-secrets.env") | ||
| if err := os.Chmod(path, 0o600); err != nil { |
There was a problem hiding this comment.
internal/deploy/secrets.go:31: pullAndRunTools starts the tools container without --user, so on a Linux VPS its bind-mounted output is normally owned by the image user (commonly root), not by the CLI user. That user can remove the file through the caller-owned directory but cannot chmod it, causing every Configure to abort here before secret resolution unless the image happens to arrange matching ownership.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
…(root-owned files on Linux VPS)
|
augment review |
|
augment review |
up.go's completion message always said https://id.versola.kz regardless of what --auth-url was actually passed -- any deployment using a different domain would be told to visit the wrong endpoint after a successful deploy (goshacodes/augment review on #7).
…Configure - OpenbaoVolumeName (tools.go) gives local/vps distinct volume names instead of one shared name across both compose templates -- was already referenced by configure.go/up.go in the previous commit without this definition existing yet, which would have broken a fresh checkout at that commit - uninstall.go updated to match - vps confirmation moved from Up to before Configure runs at all -- Configure's Finalize was already irreversible by the time Up asked (goshacodes/augment review on #7)
Volume separation alone wasn't enough -- the container_name was still shared (versola-openbao), so switching targets on one machine could find the OTHER target's container already running, leave it alone (Configure's own optimization), and never mount the freshly created target-specific volume at all -- silently resolving secrets against the wrong target's OpenBao entirely (augment review on versolauth/versola-cli#7).
Both targets' compose fragments bind OpenBao to the same host port -- separate container names/volumes don't change that. Previously this surfaced as a raw Docker 'port already allocated' failure; now it says which container to stop (augment review on #7).
* tools: add OpenBao to the local docker-local compose stack * gen-env: placeholder secrets (incl. paired public keys) for docker-local, resolved via OpenBao * feat: vps env branch in gen-env.scala (host-network defaults, OpenBao secrets) * feat: vps compose template + TARGET-aware entrypoint in versola-tools * docs: document OpenBao one-time setup for docker-local and vps * fix: restrict OpenBao's listener to loopback (was reachable on all host/VPS interfaces) * fix: bind auth/central/edge to loopback on vps (was reachable on VPS's public interface, bypassing nginx) * fix: keep edge key-id paired with its OpenBao-resolved private key; document seeding real crypto material for vps migration * fix: keep edge key-id paired with its OpenBao-resolved private key; document seeding real crypto material for vps migration * docs: warn about container-name conflict on first vps configure after a pre-existing deployment * fix: address goshacodes review on #176 - decouple target/env, require --auth-url - unify central's default port to 8090 (was 9001 in interactive mode) - split gen-env.scala's env into target (picks network defaults) and a separate env value: docker-local stays fixed, vps now reads it from ENV_NAME instead of always hardcoding prod - vps no longer defaults its public URL to our own domain -- AUTH_URL is now required, read from the environment, fails loudly if missing - Dockerfile.tools' build-time smoke test passes a placeholder AUTH_URL for the vps check * fix: require --postgres-host for vps too, same as --auth-url Postgres URL was still hardcoded to 127.0.0.1:5432 for vps -- not company-specific like the domain was, but still an infra assumption that won't hold for every future deployment (goshacodes' review on #176: 'user should provide this URL, we should not set defaults'). One POSTGRES_HOST value now drives all three services' currentSchema URLs instead of three separate hardcoded defaults. * fix: revert central's default port back to 9001 Broke ci-cd.yml's e2e job -- it hardcodes PORT=9001 when starting central directly for gen-env.scala's 'local' branch, so auth couldn't reach it after the port-unification commit changed this default to 8090 (retried until OOM). Reverted; the port-consistency comment this was addressing was about the general discrepancy, not this specific CI-load-bearing value. * fix: quote EDGE_PUBLIC_JWK in vps seeding example, matches JWKS_JSON Unquoted JSON in the docs example is subject to shell word-splitting on commas/braces -- would seed a corrupted value if copy-pasted as-is. * fix: separate OpenBao container_name per target too, not just the volume Volume separation alone wasn't enough -- the container_name was still shared (versola-openbao), so switching targets on one machine could find the OTHER target's container already running, leave it alone (Configure's own optimization), and never mount the freshly created target-specific volume at all -- silently resolving secrets against the wrong target's OpenBao entirely (augment review on versolauth/versola-cli#7). * fix: quote real secret placeholders in vps seeding examples Same class of bug as EDGE_PUBLIC_JWK earlier -- unquoted values containing \$, whitespace, *, or ! would be mangled by the shell before bao receives them. * fix: drop -t from the policy-write docker exec, heredoc stdin isn't a TTY docker exec -it fails to allocate a pseudo-TTY when stdin is actually a heredoc, not a real terminal -- step 5 (policy write) would fail before ever reaching the policy content, leaving the AppRole role created in step 6 with no working policy attached.
* refactor(bootstrap): split into configure/up, add state.json - internal/deploy: Configure (checks + versola-tools + compose.yml) and Up (start stack, wait readiness) as separate steps, bootstrap.local calls both in sequence -- external behavior unchanged. - internal/state: replace bare 'version' file with state.json (target/version/configuredAt/migratedAt), keep reading the old format for deployments made by earlier CLI builds. - each configure run gets its own bundle-<timestamp> directory instead of reusing one fixed path -- works around a Docker Desktop bug where bind-mounting a wiped-and-recreated path can make cp fail with 'File exists' for a file that doesn't exist. - internal/docker: docker.Run/Cmd extracted out of bootstrap.go, reused by uninstall.go. * feat: OpenBao-backed secrets, vps target support (#7) * deploy: create the external openbao-file volume before compose up * secrets: OpenBao AppRole client + secrets login/test commands * deploy: resolve auth/central/edge secrets against OpenBao before starting the stack * feat: support vps target in configure/up (TARGET passthrough, confirmation before touching prod) * fix: stable compose project name across configure runs (was failing on redeploy) * fix: validate target in openbao credentials path, remove OpenBao volume on local uninstall * fix: uninstall only clears ~/.versola/active (was wiping all targets' OpenBao credentials) * fix: prompt for OpenBao secret-id instead of passing it as an arg; PortFree does a raw bind check even for our own container * fix: stop echoing vps admin password on every deploy; preserve deployment state across a failed redeploy; don't drop unknown OpenBao keys on write; prompt for secret-id instead of passing it as an arg; raw bind check for our own container's port * fix: don't over-check own container's port on native Docker; clean up orphaned OpenBao container on uninstall; fix stale secrets-login usage hint * fix: mask secret-id input at the terminal (was visible in scrollback/session recordings) * fix: write state.json atomically (was truncate-then-write); remove generated-secrets.env candidates after resolving (world-readable, held real secret material) * fix: restrict generated-secrets.env permissions immediately, not only after successful resolve * fix: don't abort configure when chmod on generated-secrets.env fails (root-owned files on Linux VPS) * fix: create bundle directory 0700 instead of 0755 — closes the secret-exposure gap chmod couldn't (root-owned files on Linux) * feat: add --auth-url flag, pass ENV_NAME/AUTH_URL to versola-tools vps deploys now require --auth-url explicitly instead of versola's gen-env.scala hardcoding our domain -- see versolauth/versola#176. ENV_NAME=prod is now passed explicitly too, decoupled from target the same way. * feat: add --postgres-host flag, required for vps Same reasoning as --auth-url -- see versolauth/versola#176. * fix: record --auth-url in state, print it instead of hardcoded domain up.go's completion message always said https://id.versola.kz regardless of what --auth-url was actually passed -- any deployment using a different domain would be told to visit the wrong endpoint after a successful deploy (goshacodes/augment review on #7). * fix: separate OpenBao volume per target, ask vps confirmation before Configure - OpenbaoVolumeName (tools.go) gives local/vps distinct volume names instead of one shared name across both compose templates -- was already referenced by configure.go/up.go in the previous commit without this definition existing yet, which would have broken a fresh checkout at that commit - uninstall.go updated to match - vps confirmation moved from Up to before Configure runs at all -- Configure's Finalize was already irreversible by the time Up asked (goshacodes/augment review on #7) * fix: use target-specific OpenBao container name, not shared Matches the openbao-<target> container_name compose now uses. Configure checks the right one before deciding to leave it running; uninstall's orphan-cleanup checks both, since an untracked orphan's target is unknown by definition. * fix: clear error when the other target's OpenBao already holds port 8200 Both targets' compose fragments bind OpenBao to the same host port -- separate container names/volumes don't change that. Previously this surfaced as a raw Docker 'port already allocated' failure; now it says which container to stop (augment review on #7).
1 participant
CLI-side counterpart to the versola repo's OpenBao PR: auth/central/edge's
generated configs now reference secrets as
${?VAR}placeholders insteadof literal values, and this is what actually resolves them before the
stack starts. Companion PR: (versolauth/versola#176).
internal/openbao: AppRole client (login, read/write against OpenBao'sKV v2 API) and
~/.versola/openbao/<target>.jsoncredential storage.versola secrets login <target> <address> <role-id> <secret-id>/secrets test <target>: stores and verifies AppRole credentials for atarget, so
configurehas something to authenticate with.internal/deploy/secrets.go: for each of auth/central/edge, reads the*.generated-secrets.envcandidates versola-tools wrote, resolves eachkey against OpenBao (an existing value wins over the fresh candidate,
which gets stored if it's new), and writes the result as
*.secrets.envfor Compose'senv_file:to load.Configurecreates the externalversola-openbao-filevolume andstarts (or reuses an already-running)
openbaoservice beforeresolving secrets against it.
vpstarget support:Configure/Upaccept"vps"alongside"local", passTARGET=vpsthrough to versola-tools, skip thelocal-only port check and postgres/nginx compose services, and ask for
explicit confirmation before
Uptouches the real VPS (central runsmigrations against the live database as a side effect of starting, and
there's no separate
migratestep yet to review first).Verified locally:
versola bootstrap localstill works unchanged (secretsnow resolve through OpenBao instead of being written literally).
vpstarget not yet exercised against the real VPS — pending OpenBao being set
up there (see versola's develop.md).