feat(cli): address team vaults as <project> <env> - #109
Merged
Conversation
`teams push|pull|grant` took a single `<vault>` name, so a team holding more than one project had to encode both halves by hand and hope everyone spelled it the same way. They now take `<project> <env>` and join them into the `project/env` vault name. The split lives entirely in the CLI — vaultName()/splitVaultName() are the only things that know about it, and the server still stores one opaque vault name — so there's no migration. Both halves reject a "/" so the join stays unambiguous and the split is a true inverse. `teams vaults` now breaks the name back into project/env columns, falling back to the raw name for vaults created before the convention. Those legacy vaults are no longer addressable (their names don't contain a slash), so resolveVaultId() lists what the team actually has instead of just saying "not found" — better than silently retargeting a push, which in a secrets tool would write to the wrong vault. Note push/pull carry two different "env"s: the `<env>` positional is the environment half of the address, `--env` is the local .env path. Verified commander keeps them separate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
ralyodio
marked this pull request as ready for review
July 30, 2026 18:39
ralyodio
added a commit
that referenced
this pull request
Jul 30, 2026
The "Connect the CLI" card handed out:
LOGICSRC_API=https://app.logicsrc.com logicsrc login
logicsrc teams push <team> prod --env .env
logicsrc teams pull <team> prod --env .env
Two things are wrong with that, and both survived a release.
Since #109 addressed vaults as <team> <project> <env>, push and pull take
three positionals. The hint passes two, so pasting it exits with a missing-
argument error -- the card is not merely stale, it is broken.
The LOGICSRC_API prefix sets the variable to the value the CLI already
defaults to (DEFAULT_API_URL, #107), so on the hosted app it does nothing
while reading like a required step. It is now emitted only when the origin
is not the default, which is the case it exists for: self-hosting.
`--env .env` is dropped for the same reason -- it restates the option's own
default, and sitting next to the new <env> positional it made one flag and
one argument look like the same thing.
Same stale two-argument form fixed in the post-install hint (install.sh) and
the accept-invite message, and in the empty-vault-list prompt on the card.
CLI_HINT moves to src/lib/cli-hint.mjs so a test can assert on the rendered
commands without standing up express and the database, matching how the
other lib-level views are covered. The tests pin the argument count rather
than the prose: restyling the card stays free, dropping an argument does not.
apps/pwa: 13/13 pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ralyodio
added a commit
that referenced
this pull request
Jul 30, 2026
…ashes (#112) #109 addressed vaults as <project>/<env> and shipped broken: every push failed with Vault name must be lowercase letters, numbers, and dashes. The vault-create endpoint slugifies through /^[a-z0-9][a-z0-9-]{0,62}$/ (apps/pwa/src/routes/credshare.mjs), so a "/" join is refused outright. Nothing in the CLI ever saw it, because the tests exercised vaultName and splitVaultName in isolation and never made a request — the one assumption that mattered, that the server takes an arbitrary vault name, was the one left unverified. Switches the separator to "--", which is inside the allowed character set and still splits unambiguously since neither half may contain one. A single dash would not: "a-b" + "c" and "a" + "b-c" would collide. Also validates the joined name against the server's own regex before the request, so a bad name fails locally with a useful message rather than a 422 after the .env has been read. The tests now assert the produced name matches that regex, so the separator cannot drift back out of the allowed set without failing. Verified end to end against app.logicsrc.com: push, then pull into a scratch file and diff — keys and values both round-trip losslessly. Then 49 repos pushed under the profullstack team; server reports 49 vaults, 169 secrets, 0 failures. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
What
logicsrc teams push|pull|granttook a single<vault>positional. They now take<project> <env>:The two halves are joined into the
project/envvault name. CLI-only —vaultName()/splitVaultName()inpackages/cli/src/teams.tsare the only code that knows about the convention, and the server still stores a single opaque vault name. No migration, no API change.Details
/, so the join is unambiguous andsplitVaultNameis a true inverse ofvaultName.teams vaultsbreaks the stored name back intoproject/envcolumns, falling back to the raw name (env shown as—) for vaults that predate the convention.resolveVaultId()now lists the team's actual vault names in the not-found error rather than just saying "not found" — deliberately not a silent fallback to a same-named legacy vault, because in a secrets tool that would mean writing a push somewhere the caller didn't ask for.Testing
packages/cli/src/teams.test.ts— 8 cases covering the join, project/env distinctness, slash and blank rejection, round-tripping, and null on ambiguous or legacy names.Full workspace suite green (
npm test, all 19 workspaces,fail 0). Notecommandboard-web's test fails on a tree where onlybuild:clihas run — it needscommandboard-api/dist, which requiresplugin-agentmail,plugin-c0mputeandplugin-sh1ptto be built first. Pre-existing build-ordering, unrelated to this change; it passes once the full chain is built.push/pullcarry two different "env"s — the<env>positional (environment name) and--env(local .env path). Verified against commander that the positional and the option bind separately:Internally the positional is bound as
envNameto keep it distinct fromoptions.env.Docs
docs/credential-sharing.mdupdated to the three-positional form.🤖 Generated with Claude Code