From d9525ec1cf9aed9229220596cd38de31254aad00 Mon Sep 17 00:00:00 2001 From: claude-hdb Date: Sun, 12 Jul 2026 20:55:04 +0000 Subject: [PATCH] feat: assert the token's team before touching Coolify (fail-closed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coolify API tokens are team-scoped, and a wrong-team token does not error: the API resolves what it cannot see to `null` (getResourceByUuid walks resource → environment → project → team_id and returns null on a mismatch). To cast, `null` is indistinguishable from "this resource does not exist yet" — an invitation to create it. So an apply with a token minted under the wrong team would not fail loudly; it would provision a duplicate set of resources into the wrong team, against whatever server that team owns. Silent, mutating, discovered late. That makes this a correctness bug, not hardening. - environments.yaml carries a required `team:` per environment (id, name, or both). Required is the point: an environment with no declared team is one cast cannot verify it is pointed at. - Every command that reaches a live Coolify (apply, diff, server add, smoke) resolves GET /teams/current — the only endpoint that answers "what team does this token act as?" — and aborts on mismatch before its first READ, not merely its first write: a wrong-team diff reports "everything is absent", which is the very lie an apply would then act on. - server add and smoke take --env for this reason. A server belongs to exactly one team forever (no pivot, no is_system_wide escape hatch), and smoke writes env vars onto a live app. - New read-only `cast team` prints the token's team, so the binding can be filled in without a chicken-and-egg. With --env it also checks the binding: the dry run for "would apply refuse?". Team id 0 is a first-class value, not a falsy absent — it is the Root Team that a single-admin instance keeps everything in (app/Models/User.php). Also records the #4 investigation in docs/semantics.md: GithubApp `is_system_wide` IS the supported way to serve every team — list_github_apps scopes to `team_id = token's team OR is_system_wide`, and POST /github-apps accepts the flag — so per-team App duplication is unnecessary. Corollary: resolving a GitHub App by name is NOT a proxy for being in the right team, which is the second reason the assert has to be explicit. Closes #9 Co-Authored-By: Claude Opus 4.8 --- README.md | 53 ++++++++++++-- docs/semantics.md | 60 ++++++++++++++++ src/bindings.ts | 38 +++++++++- src/cli.ts | 90 +++++++++++++++++++++--- src/coolify.ts | 26 +++++++ src/team.ts | 76 ++++++++++++++++++++ test/cli.test.ts | 22 ++++++ test/coolify.test.ts | 17 +++++ test/fixtures/environments.yaml | 5 +- test/manifest.test.ts | 60 ++++++++++++++++ test/team.test.ts | 121 ++++++++++++++++++++++++++++++++ 11 files changed, 552 insertions(+), 16 deletions(-) create mode 100644 src/team.ts create mode 100644 test/team.test.ts diff --git a/README.md b/README.md index fb7540d..22990ba 100644 --- a/README.md +++ b/README.md @@ -47,8 +47,9 @@ infrastructure can be re-pointed at a new Coolify without touching a product. **2. A state directory** — private, yours: ``` -environments.yaml # bindings: which server each env deploys onto, the S3 - # destination, GitHub App name, smoke target, guards +environments.yaml # bindings: the team each env's token must belong to, + # which server it deploys onto, the S3 destination, + # GitHub App name, smoke target, guards secrets/..env.age # age-encrypted values for the ${…} placeholders .coolify.env # COOLIFY_BASE_URL + COOLIFY_ACCESS_TOKEN (never commit) ``` @@ -60,8 +61,9 @@ Pass it with `--state `, or set `CAST_STATE`. Defaults to the cwd. ```sh cast apply / --env [--path ] [--hostname-overlay ] cast diff / --env [--full] -cast server add --ip --key [--user root] [--port 22] -cast smoke +cast server add --ip --key --env [--user root] [--port 22] +cast smoke --env +cast team [--env ] ``` - **`apply`** — idempotent create-or-update of every manifest resource, then @@ -76,6 +78,13 @@ cast smoke endpoint still *upserts* rather than replacing. Run it after every Coolify upgrade — `apply`'s never-delete guarantee rests on that behavior, and the published OpenAPI does not describe it accurately. +- **`team`** — prints the team the configured token acts as. With `--env`, also + checks it against that environment's `team:` binding and exits non-zero on a + mismatch — the dry run for "would `apply` refuse?", answered without touching + anything. + +Every command that reaches a live Coolify takes an `--env`, because every one of +them first asserts the token's team (below). `--hostname-overlay` swaps domains for a pre-flight run against temporary hostnames; re-applying **without** it is the cutover. @@ -102,6 +111,41 @@ password manager and pass it per apply. The state directory holds ciphertext. It must never hold the identity that opens it. +## Teams: the one assert cast makes before it touches anything + +Coolify API tokens are **team-scoped**, and a token pointed at another team's +resources **does not error**. The API resolves what the token cannot see to +`null` — and to a tool like cast, `null` is indistinguishable from *"this +resource does not exist yet"*, which is an invitation to create it. An `apply` +run with a wrong-team token would not fail; it would silently provision a +**duplicate set of resources into the wrong team**, on whatever server that team +owns. Silent, mutating, discovered late. + +So every environment declares the team its token must belong to, and cast +refuses to do anything at all until it has checked: + +```yaml +environments: + prod: + server: prod-box + team: { id: 1, name: heavy-duty } +``` + +Give `id`, `name`, or both — both are compared when both are given. `id` is the +true identity (names can be renamed); `name` is what makes the file readable. +Run `cast team` to print the values for the token you currently have configured. + +The check is **fail-closed**: an environment with no `team:` is one whose token +cannot be verified, so it is a schema error, not a warning. It runs before the +first *read*, not merely before the first write — an unasserted `diff` against +the wrong team would report "everything is absent", which is precisely the lie +that an `apply` would then act on. + +Nothing below the team scopes a token. A Coolify environment has no team of its +own (it hangs off a project) and no API path scopes by one: **Coolify +environments are an organizational construct, not an auth boundary.** The team +is the only boundary there is, so it is the one cast asserts. + ## Guarding an environment An environment may refuse variables by name pattern: @@ -110,6 +154,7 @@ An environment may refuse variables by name pattern: environments: prod: server: prod-box + team: { id: 1, name: heavy-duty } forbidden_var_patterns: ["^ALLOW_"] ``` diff --git a/docs/semantics.md b/docs/semantics.md index 726426c..bff133b 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -8,6 +8,66 @@ was born in; the Coolify-source citations were verified against The command surface itself is in the README; this file is the behavior behind it. +## Team scoping + +**A Coolify API token is scoped to exactly one team, and nothing below the team +scopes it.** `User::createToken` overrides Sanctum's and stamps the session's +team onto the token (`'team_id' => session('currentTeam')->id`); the API then +resolves every request through it — `getResourceByUuid($uuid, +getTeamIdFromToken())`, which walks `resource → environment → project → +team_id`. + +The consequence that matters: **a wrong-team token does not error.** +`getResourceByUuid` returns `null` on a team mismatch, and `null` is +indistinguishable from *"this resource does not exist yet"* — which, to `apply`, +is an invitation to **create** it. An apply run with a token minted under the +wrong team would not fail loudly; it would provision a duplicate set of +resources into the wrong team, against whatever server that team owns. That is +why the team assert is a correctness guarantee and not a hardening nicety, and +why it is **fail-closed**: + +- Every environment in `environments.yaml` **must** declare `team:` (`id`, + `name`, or both). A missing team is a schema error — an environment whose + token cannot be verified is exactly the failure the binding exists to prevent. +- Every command that reaches a live Coolify (`apply`, `diff`, `server add`, + `smoke`) resolves `GET /teams/current` — the only endpoint that answers *"what + team does this token act as?"*, resolved from the token itself + (`TeamController@current_team` → `getTeamIdFromToken()`) — and compares it to + the binding **before its first read**, aborting on mismatch. Before the first + *read*, not merely the first write: a wrong-team `diff` reports "everything is + absent", which is the very lie an `apply` would then act on. +- An unreadable or unauthorized answer aborts too. It is not "no team"; it is an + unknown answer to the one question cast must not guess at. + +**An Environment is not an auth boundary.** It has no `team_id` of its own (it +belongs to a project) and no API path scopes by it — Coolify environments are an +organizational construct. The team is the only boundary there is. + +**A server belongs to exactly one team.** There is no pivot table and — unlike +`GithubApp` — no `is_system_wide` escape hatch; upstream confirms teams cannot +share a server and defers it to v5 (coollabsio/coolify#1820, #3235). Registering +a server under the wrong team is not fixable with a PATCH, which is why +`server add` takes `--env` and inherits the same assert. + +**GitHub Apps, unlike servers, *can* be shared across teams** — +`is_system_wide` is the supported mechanism, on both the read and write side: + +```php +// GithubController@list_github_apps (backs GET /github-apps) +$githubApps = GithubApp::where(function ($query) use ($teamId) { + $query->where('team_id', $teamId) + ->orWhere('is_system_wide', true); + // … +``` + +`POST /github-apps` validates and accepts `is_system_wide` (boolean), stamping +`team_id` from the token. So **one App flagged system-wide is visible and usable +from every team, and per-team App duplication is unnecessary.** Note the +corollary for cast: because `GET /github-apps` deliberately includes other +teams' system-wide Apps, **resolving a GitHub App by name is not a proxy for +being in the right team** — which is the second reason the team assert has to be +explicit. + **`dockercompose` build pack** (compose apps, box-B parity): a manifest application whose `build.pack` is `dockercompose` declares `build.compose_file` (path to the compose file in the checkout) and diff --git a/src/bindings.ts b/src/bindings.ts index bdce516..0c51232 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -2,12 +2,42 @@ import { readFileSync } from "node:fs"; import { parse } from "yaml"; import { z } from "zod"; +// The team an environment's token MUST belong to. Give `id` (the true auth +// identity — names can be renamed or duplicated), `name` (human-readable, +// greppable), or both; both are checked when both are given. `cast team` +// prints the current token's id and name so this can be filled in. +const TeamSchema = z + .object({ + // Non-negative, NOT positive: team **0 is the Root Team** — the one the + // first user of an instance gets (`if ($user->id === 0) { $team['id'] = 0; + // $team['name'] = 'Root Team'; }`, app/Models/User.php @ v4.1.2). On a + // single-admin Coolify that is the team everything lives in, so rejecting + // 0 would make the id check — the strong half of the assert — unusable on + // exactly the topology that most needs it. + id: z.number().int().nonnegative().optional(), + name: z.string().min(1).optional(), + }) + .strict() + .refine((t) => t.id !== undefined || t.name !== undefined, { + message: "team must give at least one of `id` or `name`", + }); + const BindingsSchema = z .object({ environments: z.record( z .object({ server: z.string(), + // REQUIRED, and deliberately so: this is what makes the team assert + // fail-closed (see team.ts). An environment with no declared team + // is an environment cast cannot verify it is pointed at — and an + // unverifiable target is exactly the silent-duplicate-into-the- + // wrong-team failure this binding exists to prevent. No team, no + // apply. Today one state dir holds one token (.coolify.env), so + // every environment in it normally names the same team; declaring + // it per environment keeps each one's expectation explicit and + // survives a future split into per-environment tokens. + team: TeamSchema, s3_destination: z.string().optional(), // Var-name patterns this environment refuses outright (see // assertEnvVarPolicy). Operator-owned guard: prod typically bans @@ -23,8 +53,12 @@ const BindingsSchema = z export type Bindings = z.infer; -export function loadBindings(path: string): Bindings { - const result = BindingsSchema.safeParse(parse(readFileSync(path, "utf8"))); +export function loadBindings( + path: string, + opts: { overrideText?: string } = {}, +): Bindings { + const text = opts.overrideText ?? readFileSync(path, "utf8"); + const result = BindingsSchema.safeParse(parse(text)); if (!result.success) { throw new Error(`invalid bindings ${path}: ${result.error.message}`); } diff --git a/src/cli.ts b/src/cli.ts index 6dab2b5..df3d668 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -18,14 +18,21 @@ import { desiredFromManifest, resolveCheckout } from "./resolve.js"; import { decryptSecrets, keyFileFor, secretsFileFor } from "./secrets.js"; import { serverAdd } from "./server.js"; import { smoke } from "./smoke.js"; +import { assertTeam, formatTeam } from "./team.js"; const USAGE = `usage: cast apply / --env [--path ] [--hostname-overlay ] cast diff / --env [--full] - cast server add --ip --key [--user root] [--port 22] - cast smoke + cast server add --ip --key --env [--user root] [--port 22] + cast smoke --env + cast team [--env ] --state the state checkout holding environments.yaml, secrets/ and - .coolify.env (default: $CAST_STATE, else the cwd)`; + .coolify.env (default: $CAST_STATE, else the cwd) + --env the environment to act on. Every command that reaches a live + Coolify takes one, because every one of them first asserts + the token belongs to that environment's declared team. + \`cast team\` alone (no --env) reports the token's team + without needing a binding — use it to fill environments.yaml.`; // cast is stateless: every instance-scoped input is read from the state // directory it is pointed at, never from a location the tool itself knows. @@ -248,6 +255,14 @@ async function main(): Promise { } const { baseUrl, token } = loadCoolifyEnv(join(stateDir, ".coolify.env")); const client = new CoolifyClient(baseUrl, token); + // Fail-closed, before the first live read — not merely before the first + // write. A wrong-team token makes fetchLive come back empty (the API + // resolves what it cannot see to null), so an unasserted `diff` would + // cheerfully report "everything is absent" and an unasserted `apply` + // would then create all of it in the wrong team. The read is already + // the lie; gate it, not just the write. + const team = await assertTeam(client, binding.team, envName); + console.log(`team ${formatTeam(team)} ✓`); const mode = command === "apply" || values.full ? "full" : "structural"; const live = await fetchLive(client, repoShort, envName); if (mode === "full") { @@ -306,19 +321,34 @@ async function main(): Promise { options: { ip: { type: "string" }, key: { type: "string" }, + env: { type: "string" }, user: { type: "string" }, port: { type: "string" }, state: { type: "string" }, }, }); - if (!positionals[0] || !values.ip || !values.key) { + // --env is required: a server is registered under the token's team and + // belongs to exactly one team forever (Coolify has no pivot and no + // is_system_wide escape hatch for servers). Registering it under the + // wrong team is not a mistake you fix with a PATCH — you delete and + // re-add. So it takes the same assert as every other command, against + // the team of the environment the server is being registered to serve. + if (!positionals[0] || !values.ip || !values.key || !values.env) { console.error(USAGE); return 2; } - const { baseUrl, token } = loadCoolifyEnv( - join(stateDirFrom(values.state), ".coolify.env"), - ); - await serverAdd(new CoolifyClient(baseUrl, token), { + const stateDir = stateDirFrom(values.state); + const binding = loadBindings(join(stateDir, "environments.yaml")) + .environments[values.env]; + if (!binding) { + console.error(`environment ${values.env} not in environments.yaml`); + return 2; + } + const { baseUrl, token } = loadCoolifyEnv(join(stateDir, ".coolify.env")); + const client = new CoolifyClient(baseUrl, token); + const team = await assertTeam(client, binding.team, values.env); + console.log(`team ${formatTeam(team)} ✓`); + await serverAdd(client, { name: positionals[0], ip: values.ip, keyFile: values.key, @@ -331,12 +361,27 @@ async function main(): Promise { const { values } = parseArgs({ args: rest, allowPositionals: true, - options: { state: { type: "string" } }, + options: { state: { type: "string" }, env: { type: "string" } }, }); + // smoke writes: it POSTs two env vars onto the live smoke_target app and + // deletes them again. That is a mutation, so it takes the assert like any + // other. Without it, a wrong-team token that happened to own an app of + // the same name would have that app written to instead. + if (!values.env) { + console.error(USAGE); + return 2; + } const stateDir = stateDirFrom(values.state); const { baseUrl, token } = loadCoolifyEnv(join(stateDir, ".coolify.env")); const client = new CoolifyClient(baseUrl, token); const bindings = loadBindings(join(stateDir, "environments.yaml")); + const binding = bindings.environments[values.env]; + if (!binding) { + console.error(`environment ${values.env} not in environments.yaml`); + return 2; + } + const team = await assertTeam(client, binding.team, values.env); + console.log(`team ${formatTeam(team)} ✓`); if (!bindings.smoke_target) { console.error( "environments.yaml: smoke_target (app name) required for smoke", @@ -356,6 +401,33 @@ async function main(): Promise { await smoke(client, target.uuid); return 0; } + if (command === "team") { + const { values } = parseArgs({ + args: rest, + allowPositionals: true, + options: { state: { type: "string" }, env: { type: "string" } }, + }); + const stateDir = stateDirFrom(values.state); + const { baseUrl, token } = loadCoolifyEnv(join(stateDir, ".coolify.env")); + const client = new CoolifyClient(baseUrl, token); + // Read-only, and the one command that deliberately does NOT require a + // team binding: it is how you discover the values to write into + // environments.yaml in the first place. Asserting here would be circular. + // With --env it also checks the binding, which makes it the dry run for + // "will apply refuse?" — ask the question without touching anything. + const actual = await client.currentTeam(); + console.log(`token's team: ${formatTeam(actual)}`); + if (!values.env) return 0; + const binding = loadBindings(join(stateDir, "environments.yaml")) + .environments[values.env]; + if (!binding) { + console.error(`environment ${values.env} not in environments.yaml`); + return 2; + } + await assertTeam(client, binding.team, values.env); + console.log(`matches the team ${values.env} expects ✓`); + return 0; + } console.error(USAGE); return 2; } diff --git a/src/coolify.ts b/src/coolify.ts index ee1f744..6bbd117 100644 --- a/src/coolify.ts +++ b/src/coolify.ts @@ -1,5 +1,9 @@ type Json = Record | unknown[] | null; +// The team a token acts as. Coolify's `Team` model carries more than this +// (description, personal_team, timestamps); cast only ever needs identity. +export type Team = { id: number; name: string }; + // Thrown by req/reqText on a non-2xx response. `status` lets callers narrow // handling (e.g. "treat 404 as absent, rethrow everything else") via // `instanceof HttpError` without parsing the message string; the message @@ -64,6 +68,28 @@ export class CoolifyClient { return this.reqText("GET", "/version"); } + // The team this TOKEN acts as — the question every mutation depends on + // (see team.ts for why). GET /teams/current resolves it from the token + // itself, not from a session: TeamController@current_team calls + // getTeamIdFromToken() and 404s if that team is gone + // (coollabsio/coolify v4.1.2). It is the only endpoint that answers it. + async currentTeam(): Promise { + const raw = (await this.get("/teams/current")) as Record< + string, + unknown + > | null; + const id = raw?.id; + const name = raw?.name; + // A shape we can't read is not "no team" — it's an unknown answer to the + // one question we must not guess at. Fail rather than degrade. + if (typeof id !== "number" || typeof name !== "string") { + throw new Error( + `GET /teams/current returned no usable team identity: ${JSON.stringify(raw)}`, + ); + } + return { id, name }; + } + private async resolve( kind: string, listPath: string, diff --git a/src/team.ts b/src/team.ts new file mode 100644 index 0000000..a1181f4 --- /dev/null +++ b/src/team.ts @@ -0,0 +1,76 @@ +import type { CoolifyClient, Team } from "./coolify.js"; + +// What environments.yaml declares a token must be, for a given environment. +// At least one of id/name is present (enforced by the bindings schema); both +// are compared when both are given. +export type TeamExpectation = { id?: number; name?: string }; + +export function formatTeam(t: TeamExpectation | Team): string { + const parts: string[] = []; + if (t.id !== undefined) parts.push(`id=${t.id}`); + if (t.name !== undefined) parts.push(`name=${JSON.stringify(t.name)}`); + return parts.join(" "); +} + +// The fail-closed pre-flight gate. Every command that reaches a live Coolify +// runs this BEFORE its first read, and never mutates without it. +// +// Why this has to exist at all: Coolify API tokens are team-scoped +// (User::createToken stamps `team_id` onto the token), and a token used +// against another team's resources does not error — the API resolves through +// `getResourceByUuid($uuid, getTeamIdFromToken())`, which simply returns +// `null` on a team mismatch. To cast, `null` is indistinguishable from "this +// resource does not exist yet", which is an invitation to CREATE it. So a +// wrong-team token would not fail an apply; it would silently provision a +// duplicate set of resources into the wrong team, against whatever server +// that team owns. Silent, mutating, and discovered late — the worst shape of +// failure there is. +// +// Nothing below the team scopes a token: an Environment has no `team_id` of +// its own (it hangs off a project), and no API path scopes by environment. +// Coolify environments are an organizational construct, not an auth boundary. +// The team is the only boundary there is, so it is the one thing we assert. +export async function assertTeam( + client: CoolifyClient, + expected: TeamExpectation, + envName: string, +): Promise { + // An expectation naming neither id nor name would compare nothing and pass + // against any team alive — the gate would fail OPEN. The bindings schema + // already rejects that shape, but this is the one function whose entire job + // is to fail closed, and it must not depend on a `.refine()` in another file + // to do it. Cheap, and it means no future caller can quietly defeat it. + if (expected.id === undefined && expected.name === undefined) { + throw new Error( + `cannot verify the token's team for environment ${envName}: its \`team:\` binding names neither an id nor a name`, + ); + } + const actual = await client.currentTeam(); + const mismatched: string[] = []; + if (expected.id !== undefined && expected.id !== actual.id) { + mismatched.push("id"); + } + if (expected.name !== undefined && expected.name !== actual.name) { + mismatched.push("name"); + } + if (mismatched.length === 0) return actual; + throw new Error( + [ + "refusing to touch Coolify: this token belongs to the wrong team", + "", + ` environment: ${envName}`, + ` expected team: ${formatTeam(expected)} (environments.yaml)`, + ` token's team: ${formatTeam(actual)} (GET /teams/current)`, + ` mismatched: ${mismatched.join(", ")}`, + "", + "Coolify tokens are team-scoped, and a wrong-team token does NOT fail —", + "it resolves every resource it cannot see to null. cast would read that", + 'as "does not exist yet" and create a DUPLICATE set of resources in the', + "wrong team, on whatever server that team owns.", + "", + "Fix the token (mint one from the expected team in Coolify → Keys &", + "Tokens) or fix the environment's `team:` binding — whichever is wrong.", + "`cast team` prints the team the current token acts as.", + ].join("\n"), + ); +} diff --git a/test/cli.test.ts b/test/cli.test.ts index 2af8dc6..1e8fb76 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -32,4 +32,26 @@ describe("infra cli", () => { expect(r.code).not.toBe(0); expect(r.output).toMatch(/usage: cast (apply|diff)/i); }); + // Both of these reach a live Coolify and mutate — `server add` registers a + // server into the token's team (permanently: a server belongs to exactly one + // team), and `smoke` writes env vars onto a live app. Neither may run without + // an environment to assert the token's team against. + it("refuses server add without --env, exit non-zero", () => { + const r = runCli([ + "server", + "add", + "prod-box", + "--ip", + "10.0.0.1", + "--key", + "/tmp/k", + ]); + expect(r.code).not.toBe(0); + expect(r.output).toMatch(/--env/); + }); + it("refuses smoke without --env, exit non-zero", () => { + const r = runCli(["smoke"]); + expect(r.code).not.toBe(0); + expect(r.output).toMatch(/--env/); + }); }); diff --git a/test/coolify.test.ts b/test/coolify.test.ts index 3a73001..852ba43 100644 --- a/test/coolify.test.ts +++ b/test/coolify.test.ts @@ -36,6 +36,23 @@ describe("CoolifyClient", () => { const c = new CoolifyClient("https://coolify.test", "tok", mockFetch({})); await expect(c.get("/projects")).rejects.toThrow(/GET \/projects → 404/); }); + it("reads the token's team from /teams/current", async () => { + const c = new CoolifyClient( + "https://coolify.test", + "tok", + mockFetch({ + "GET /api/v1/teams/current": { + id: 1, + name: "heavy-duty", + personal_team: false, + }, + }), + ); + await expect(c.currentTeam()).resolves.toEqual({ + id: 1, + name: "heavy-duty", + }); + }); it("reads version as plain text, not JSON", async () => { const fetchImpl = vi.fn( async () => new Response("4.1.2", { status: 200 }), diff --git a/test/fixtures/environments.yaml b/test/fixtures/environments.yaml index b0e03f8..da81dbb 100644 --- a/test/fixtures/environments.yaml +++ b/test/fixtures/environments.yaml @@ -1,8 +1,11 @@ environments: prod: server: prod-box + team: { id: 1, name: heavy-duty } s3_destination: s3-backups forbidden_var_patterns: ["^ALLOW_"] - staging: { server: staging-vm } + staging: + server: staging-vm + team: { id: 1, name: heavy-duty } github_apps: widget: my-github-app diff --git a/test/manifest.test.ts b/test/manifest.test.ts index bcc021a..04d974f 100644 --- a/test/manifest.test.ts +++ b/test/manifest.test.ts @@ -135,4 +135,64 @@ describe("loadBindings", () => { expect(b.environments.prod.forbidden_var_patterns).toEqual(["^ALLOW_"]); expect(b.environments.staging.forbidden_var_patterns).toBeUndefined(); }); + it("carries an environment's expected team through", () => { + const b = loadBindings(`${FIX}environments.yaml`); + expect(b.environments.prod.team).toEqual({ id: 1, name: "heavy-duty" }); + }); + // Fail-closed at the schema: an environment with no declared team is one + // whose token cannot be verified, and an unverifiable target is exactly the + // duplicate-into-the-wrong-team failure the binding exists to prevent. + it("rejects an environment with no team", () => { + expect(() => + loadBindings(`${FIX}environments.yaml`, { + overrideText: ` +environments: + prod: { server: prod-box } +github_apps: { widget: my-github-app } +`, + }), + ).toThrow(/team/); + }); + it("rejects a team that names neither id nor name", () => { + expect(() => + loadBindings(`${FIX}environments.yaml`, { + overrideText: ` +environments: + prod: { server: prod-box, team: {} } +github_apps: { widget: my-github-app } +`, + }), + ).toThrow(/at least one of/); + }); + // Coolify's Root Team is id 0 (app/Models/User.php @ v4.1.2) — the team a + // single-admin instance keeps everything in. Rejecting it would make the id + // check unusable on exactly the topology that most needs it. + it("accepts team id 0, the Root Team", () => { + const b = loadBindings(`${FIX}environments.yaml`, { + overrideText: ` +environments: + prod: { server: prod-box, team: { id: 0, name: Root Team } } +github_apps: { widget: my-github-app } +`, + }); + expect(b.environments.prod.team).toEqual({ id: 0, name: "Root Team" }); + }); + it("accepts a team given by id alone, or by name alone", () => { + const byId = loadBindings(`${FIX}environments.yaml`, { + overrideText: ` +environments: + prod: { server: prod-box, team: { id: 2 } } +github_apps: { widget: my-github-app } +`, + }); + expect(byId.environments.prod.team).toEqual({ id: 2 }); + const byName = loadBindings(`${FIX}environments.yaml`, { + overrideText: ` +environments: + prod: { server: prod-box, team: { name: heavy-duty } } +github_apps: { widget: my-github-app } +`, + }); + expect(byName.environments.prod.team).toEqual({ name: "heavy-duty" }); + }); }); diff --git a/test/team.test.ts b/test/team.test.ts new file mode 100644 index 0000000..9f81b26 --- /dev/null +++ b/test/team.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it, vi } from "vitest"; +import { CoolifyClient } from "../src/coolify.js"; +import { assertTeam } from "../src/team.js"; + +function clientReturning(team: unknown, status = 200): CoolifyClient { + const fetchImpl = vi.fn( + async () => new Response(JSON.stringify(team), { status }), + ) as unknown as typeof fetch; + return new CoolifyClient("https://coolify.test", "tok", fetchImpl); +} + +const HEAVY_DUTY = { id: 1, name: "heavy-duty", personal_team: false }; + +describe("assertTeam", () => { + it("passes when both id and name match, returning the live team", async () => { + const team = await assertTeam( + clientReturning(HEAVY_DUTY), + { id: 1, name: "heavy-duty" }, + "prod", + ); + expect(team).toEqual({ id: 1, name: "heavy-duty" }); + }); + + it("passes on id alone, and on name alone", async () => { + await expect( + assertTeam(clientReturning(HEAVY_DUTY), { id: 1 }, "prod"), + ).resolves.toBeTruthy(); + await expect( + assertTeam(clientReturning(HEAVY_DUTY), { name: "heavy-duty" }, "prod"), + ).resolves.toBeTruthy(); + }); + + // The whole point of the issue: a token minted under another team must turn + // a silent mis-target into a refusal, because Coolify itself would not + // error — it would resolve every resource to null and invite a duplicate + // create in the wrong team. + it("refuses on an id mismatch, naming both teams and the environment", async () => { + const err = await assertTeam( + clientReturning({ id: 3, name: "personal" }), + { id: 1, name: "heavy-duty" }, + "prod", + ).catch((e: Error) => e); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/wrong team/); + expect((err as Error).message).toMatch(/environment:\s+prod/); + expect((err as Error).message).toMatch(/expected team:\s+id=1/); + expect((err as Error).message).toMatch(/token's team:\s+id=3/); + expect((err as Error).message).toMatch(/mismatched:\s+id, name/); + }); + + // A renamed-but-same-id team, or an id typo against a right-named team: + // either half mismatching is a refusal. Both are compared when both given. + it("refuses when only the name mismatches", async () => { + await expect( + assertTeam( + clientReturning({ id: 1, name: "some-other-team" }), + { id: 1, name: "heavy-duty" }, + "prod", + ), + ).rejects.toThrow(/mismatched:\s+name/); + }); + + it("refuses when only the id mismatches", async () => { + await expect( + assertTeam( + clientReturning({ id: 9, name: "heavy-duty" }), + { id: 1, name: "heavy-duty" }, + "prod", + ), + ).rejects.toThrow(/mismatched:\s+id/); + }); + + // An unreadable answer is not "no team" — it is an unknown answer to the one + // question we must not guess at, so it fails rather than degrading to a pass. + it("refuses when /teams/current has no usable identity", async () => { + await expect( + assertTeam( + clientReturning({ message: "Unauthenticated." }), + { id: 1 }, + "prod", + ), + ).rejects.toThrow(/no usable team identity/); + }); + + it("surfaces a token rejection rather than swallowing it", async () => { + await expect( + assertTeam( + clientReturning({ message: "bad token" }, 401), + { id: 1 }, + "prod", + ), + ).rejects.toThrow(/GET \/teams\/current → 401/); + }); + + // Team 0 is the Root Team — the team the first user of an instance gets + // (app/Models/User.php @ v4.1.2). On a single-admin Coolify it is the team + // everything lives in, so `0` must be a first-class expectation, not a + // falsy value that quietly compares as absent. + it("compares team id 0 (the Root Team) as a real expectation", async () => { + const root = { id: 0, name: "Root Team" }; + await expect( + assertTeam(clientReturning(root), { id: 0 }, "prod"), + ).resolves.toEqual(root); + await expect( + assertTeam( + clientReturning({ id: 3, name: "personal" }), + { id: 0 }, + "prod", + ), + ).rejects.toThrow(/wrong team/); + }); + + // The gate must fail closed on its own, without leaning on the bindings + // schema to have rejected an empty team first: an expectation that names + // nothing would compare nothing and pass against ANY team. + it("refuses an expectation that names neither id nor name", async () => { + await expect( + assertTeam(clientReturning({ id: 99, name: "anything" }), {}, "prod"), + ).rejects.toThrow(/names neither an id nor a name/); + }); +});