From e1e5e80992dda38315c921f326b8dbc9c3388a01 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Thu, 30 Jul 2026 19:51:30 +0000 Subject: [PATCH] =?UTF-8?q?fix(cli):=20vault=20names=20join=20with=20"--",?= =?UTF-8?q?=20not=20"/"=20=E2=80=94=20the=20server=20rejects=20slashes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #109 addressed vaults as / 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) --- docs/credential-sharing.md | 4 ++- packages/cli/src/index.ts | 4 +-- packages/cli/src/teams.test.ts | 65 +++++++++++++++++++++++++++------- packages/cli/src/teams.ts | 40 ++++++++++++++------- 4 files changed, 86 insertions(+), 27 deletions(-) diff --git a/docs/credential-sharing.md b/docs/credential-sharing.md index 1dc8960..6d830d6 100644 --- a/docs/credential-sharing.md +++ b/docs/credential-sharing.md @@ -183,7 +183,9 @@ re-wraps (seals) it to the new member's public key. The private key lives only i logicsrc login # Owner: create a team, push a local .env into an encrypted vault, invite people. -# A vault is addressed as , stored as the vault name project/env. +# A vault is addressed as , stored as the vault name +# project--env (a double dash: the server slugs vault names through +# /^[a-z0-9][a-z0-9-]{0,62}$/, so a "/" would be rejected). logicsrc teams create acme --name "Acme Inc" logicsrc teams push acme web prod --env .env # encrypt + upload logicsrc teams invite acme teammate@example.com # emails an accept link diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 60613bf..3ae710e 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -615,7 +615,7 @@ teams .argument("", "Environment name (prod, staging, …)") .option("--env ", "Source .env file", ".env") .option("--format ", "table, json, or markdown", "table") - .description("Encrypt and push a local .env into a team vault (/).") + .description("Encrypt and push a local .env into a team vault (--).") .action((slug, project, env, options) => teamsPushAction(slug, project, env, { env: options.env, format: options.format as OutputFormat })); teams @@ -625,7 +625,7 @@ teams .argument("", "Environment name (prod, staging, …)") .option("--env ", "Destination .env file", ".env") .option("--format ", "table, json, or markdown", "table") - .description("Pull a team vault (/) and decrypt it into a local .env.") + .description("Pull a team vault (--) and decrypt it into a local .env.") .action((slug, project, env, options) => teamsPullAction(slug, project, env, { env: options.env, format: options.format as OutputFormat })); const accounts = program.command("accounts").description("Manage connected social and email accounts."); diff --git a/packages/cli/src/teams.test.ts b/packages/cli/src/teams.test.ts index c606407..e9a2433 100644 --- a/packages/cli/src/teams.test.ts +++ b/packages/cli/src/teams.test.ts @@ -1,14 +1,32 @@ import { describe, expect, it } from "vitest"; -import { splitVaultName, vaultName } from "./teams.js"; +import { splitVaultName, vaultName, VAULT_SEP } from "./teams.js"; // A vault is addressed as on the command line and stored as a -// single `project/env` name server-side. The join is the only thing keeping -// those two halves apart, so it has to reject anything that would make the -// name ambiguous — a wrong split would point a push at the wrong vault. +// single name server-side. Two things have to hold: the join must survive the +// server's own validation, and it must split back unambiguously — a wrong split +// would point a push at the wrong vault. +// +// The server slugifies vault names through /^[a-z0-9][a-z0-9-]{0,62}$/. The +// first cut of this used "/" as the separator, which that regex rejects, so +// every push failed with a 422 the unit tests never saw. SERVER_SLUG below is +// that regex, asserted directly, so the separator can't drift out of the +// allowed character set again without a test failing. +const SERVER_SLUG = /^[a-z0-9][a-z0-9-]{0,62}$/; describe("vaultName", () => { - it("joins project and env with a slash", () => { - expect(vaultName("web", "prod")).toBe("web/prod"); + it("joins project and env with the separator", () => { + expect(vaultName("web", "prod")).toBe(`web${VAULT_SEP}prod`); + }); + + it("produces a name the server will accept", () => { + expect(vaultName("web", "prod")).toMatch(SERVER_SLUG); + expect(vaultName("food-delivery-multivendor-enatega-multivendor-backend", "prod")).toMatch(SERVER_SLUG); + }); + + it("never uses a separator the server rejects", () => { + // The regression that shipped: "/" is not in [a-z0-9-]. + expect(VAULT_SEP).toMatch(/^[a-z0-9-]+$/); + expect(vaultName("web", "prod")).not.toContain("/"); }); it("keeps distinct envs of one project apart", () => { @@ -19,9 +37,26 @@ describe("vaultName", () => { expect(vaultName("api", "prod")).not.toBe(vaultName("web", "prod")); }); - it("rejects a slash in either half", () => { - expect(() => vaultName("web/api", "prod")).toThrow(/cannot contain/); - expect(() => vaultName("web", "prod/eu")).toThrow(/cannot contain/); + it("keeps a dashed project distinct from a dashed env", () => { + // "a-b" + "c" and "a" + "b-c" must not collide — the reason the separator + // is a double dash rather than a single one. + expect(vaultName("a-b", "c")).not.toBe(vaultName("a", "b-c")); + }); + + it("rejects the separator inside either half", () => { + expect(() => vaultName(`web${VAULT_SEP}api`, "prod")).toThrow(/cannot contain/); + expect(() => vaultName("web", `prod${VAULT_SEP}eu`)).toThrow(/cannot contain/); + }); + + it("rejects characters the server would refuse", () => { + expect(() => vaultName("web/api", "prod")).toThrow(/not a valid vault name/); + expect(() => vaultName("Web", "prod")).toThrow(/not a valid vault name/); + expect(() => vaultName("web_api", "prod")).toThrow(/not a valid vault name/); + expect(() => vaultName("-web", "prod")).toThrow(/not a valid vault name/); + }); + + it("rejects a combined name past the server's 63-character limit", () => { + expect(() => vaultName("a".repeat(60), "prod")).toThrow(/at most 63/); }); it("rejects empty or blank halves", () => { @@ -36,15 +71,21 @@ describe("splitVaultName", () => { expect(splitVaultName(vaultName("web", "prod"))).toEqual({ project: "web", env: "prod" }); }); + it("round-trips halves that contain single dashes", () => { + expect(splitVaultName(vaultName("playground-encryptfiles-web", "prod"))) + .toEqual({ project: "playground-encryptfiles-web", env: "prod" }); + }); + it("returns null for legacy single-word names", () => { // Vaults created before the split are still listable; they just don't // decompose, so `teams vaults` shows the raw name instead of guessing. expect(splitVaultName("prod")).toBeNull(); + expect(splitVaultName("web-prod")).toBeNull(); }); it("returns null rather than guessing at an ambiguous name", () => { - expect(splitVaultName("a/b/c")).toBeNull(); - expect(splitVaultName("/prod")).toBeNull(); - expect(splitVaultName("web/")).toBeNull(); + expect(splitVaultName(`a${VAULT_SEP}b${VAULT_SEP}c`)).toBeNull(); + expect(splitVaultName(`${VAULT_SEP}prod`)).toBeNull(); + expect(splitVaultName(`web${VAULT_SEP}`)).toBeNull(); }); }); diff --git a/packages/cli/src/teams.ts b/packages/cli/src/teams.ts index f96e530..f49f2e3 100644 --- a/packages/cli/src/teams.ts +++ b/packages/cli/src/teams.ts @@ -176,12 +176,22 @@ class DeviceFlowUnsupported extends Error { constructor() { super("device flow not supported by this server"); } } -// A vault is addressed as /, so one team can hold web/prod, +// A vault is addressed as , so one team can hold web/prod, // web/staging and api/prod side by side. The split lives entirely in the CLI — -// the server still stores a single opaque vault name — so this join and +// the server stores a single opaque vault name — so this join and // splitVaultName() below are the only places that know about the convention. -// Neither half may contain a slash, which keeps the join unambiguous and makes -// splitVaultName a true inverse. +// +// The separator is "--", NOT "/". The server slugifies vault names through +// /^[a-z0-9][a-z0-9-]{0,62}$/ and rejects anything else, so a "/" join is +// refused outright with "Vault name must be lowercase letters, numbers, and +// dashes." A double dash is inside the allowed character set and still splits +// unambiguously, because neither half may contain one. +export const VAULT_SEP = "--"; + +// Mirrors the server's slugify(). Enforced here so a bad name fails locally +// with a useful message instead of a 422 after the file has been read. +const VAULT_NAME = /^[a-z0-9][a-z0-9-]{0,62}$/; + export function vaultName(project: string, env: string): string { const parts: ReadonlyArray = [ ["project", project], @@ -191,20 +201,26 @@ export function vaultName(project: string, env: string): string { if (!value || !value.trim()) { throw new Error(`Missing ${label}. Usage: logicsrc teams push `); } - if (value.includes("/")) { - throw new Error(`The ${label} "${value}" cannot contain "/" — it separates project from env in a vault name.`); + if (value.includes(VAULT_SEP)) { + throw new Error(`The ${label} "${value}" cannot contain "${VAULT_SEP}" — it separates project from env in a vault name.`); } } - return `${project}/${env}`; + const name = `${project}${VAULT_SEP}${env}`; + if (!VAULT_NAME.test(name)) { + throw new Error( + `"${name}" is not a valid vault name. Project and env must be lowercase letters, numbers and dashes, and together at most 63 characters.` + ); + } + return name; } /** Inverse of vaultName; null for names that predate the convention. */ export function splitVaultName(name: string): { project: string; env: string } | null { - const slash = name.indexOf("/"); - if (slash <= 0 || slash === name.length - 1) return null; - const env = name.slice(slash + 1); - if (env.includes("/")) return null; - return { project: name.slice(0, slash), env }; + const at = name.indexOf(VAULT_SEP); + if (at <= 0) return null; + const env = name.slice(at + VAULT_SEP.length); + if (!env || env.includes(VAULT_SEP)) return null; + return { project: name.slice(0, at), env }; } async function resolveVaultId(client: TeamClient, slug: string, vault: string): Promise {