diff --git a/docs/semantics.md b/docs/semantics.md index 58cb803..2f2224f 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -180,6 +180,60 @@ softened by an implementation detail): of value** — "off" means absent, not `false` (see the README). `apply` also refuses `--path` combined with `--env prod`: prod always reads the default branch, so a feature-branch checkout can never reach it. +- **Reserved names:** cast never writes `SOURCE_COMMIT` or a `COOLIFY_*` var, + under any manifest, in any environment. See below. + +## Reserved env var names (`SOURCE_COMMIT`, `COOLIFY_*`) + +Coolify injects a set of values into an application's runtime environment +itself — `SOURCE_COMMIT`, and the `COOLIFY_*` family (`COOLIFY_URL`, +`COOLIFY_FQDN`, `COOLIFY_BRANCH`, `COOLIFY_RESOURCE_UUID`, +`COOLIFY_CONTAINER_NAME`) — and it does so behind one guard +(`app/Jobs/ApplicationDeploymentJob.php`, v4.1.2): + +```php +if ($this->application->environment_variables->where('key', 'SOURCE_COMMIT')->isEmpty()) { + $coolify_envs->put('SOURCE_COMMIT', $this->commit); +} +``` + +**Coolify skips its own injection of a name when the resource already carries an +env var of that name.** So a resource-level `SOURCE_COMMIT` does not merely fail +to help — it **suppresses** the value Coolify would otherwise have provided. An +**empty** one suppresses it exactly as completely: `isEmpty()` is asked of the +*collection of vars*, never of the value. Presence, not value — the same rule +`forbidden_var_patterns` holds to, for the same reason. + +And it **fails green**. The deploy succeeds, the health check passes, the +container runs, and the only symptom is that `/version` — which reads +`process.env.SOURCE_COMMIT` at request time, and which a production cutover is +gated on — reports `unknown`. + +Anything that writes env vars can set that trap, and cast is a thing that writes +env vars. So the rule is applied at **every** place cast touches one: + +| verb | behavior | +| --- | --- | +| `apply` / `diff` / `capture` / `inventory` | **refuse.** Any manifest read whose env template declares a reserved name fails the run, before any write, naming the var and the consequence. | +| `inventory --emit-draft` | **never copies one.** A reserved name read off a live box is dispositioned `suppressed`: kept out of the emitted template *and* out of the age store, its live value read into no artifact, and named in `UNCAPTURED.md` with the reason. | +| `diff` | **a finding, not an orphan var.** A reserved name on a live resource is *not* filed under `remove-candidate` ("apply never removes these; read them by eye"). It is not cosmetic residue — it is an active suppression — so it prints as a `FINDING`, with the consequence, and the report is **not clean**. | +| `smoke` | its probe vars are asserted to be outside the reserved space. | + +Two things this rule is **not**: + +- It is **not** `forbidden_var_patterns`. That one is *policy* — an environment's + own choice about its own vars — and it lives in the operator's private state + precisely so that a product-side change cannot lower its own guard. This one is + a **fact about Coolify**: true on every box, in every environment, for every + project. There is no environment in which declaring `SOURCE_COMMIT` is correct, + so there is no file in which it can be permitted. It lives in cast's code. +- It is **not** a deletion. `apply never deletes` holds unchanged: on a live box + that already carries one, cast **reports** it and a human removes it in the + Coolify UI. + +Nothing here applies to cast's own config vars (`COOLIFY_BASE_URL`, +`COOLIFY_ACCESS_TOKEN`, `COOLIFY_READ_ONLY`): those are read from the operator's +local instance file and are never written to a resource. ## The registry (`projects:`) @@ -541,9 +595,11 @@ flat `domains` on a Coolify 4.1.2 service), Basic Auth / custom Traefik labels, build and deploy command overrides, backup schedules (not exposed on a database's GET — **a rebuild has no backups until you declare them**), database kinds cast does not model (MySQL, MariaDB, MongoDB, KeyDB, Dragonfly, ClickHouse — named, -never silently dropped), env var names a cast template cannot express, and -applications whose build pack the manifest has no vocabulary for (left *out* of -the manifest rather than fabricated into the nearest pack). +never silently dropped), env var names a cast template cannot express, names +**reserved by the platform** (`SOURCE_COMMIT`, `COOLIFY_*` — suppressed, never +copied; see [Reserved env var names](#reserved-env-var-names-source_commit-coolify_)), +and applications whose build pack the manifest has no vocabulary for (left *out* +of the manifest rather than fabricated into the nearest pack). It also carries the table below, because that is the file someone will be reading at the worst possible moment. diff --git a/src/capture.ts b/src/capture.ts index 5359fca..18a0357 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -1,3 +1,4 @@ +import { assertNoReservedEnvNames, isReservedEnvName } from "./reserved.js"; import type { RequiredSecret } from "./resolve.js"; // What the manifest writes for a provider-generated name. Not the source box's @@ -108,6 +109,18 @@ export function classify( live: LiveEnvs, overrides: Record, ): Classification { + // The reserved-name rule, once more at the file (reserved.ts). Unreachable + // through the CLI — `requiredSecrets` refuses a manifest declaring one before + // capture is ever called — and kept anyway: it is the invariant, not the check + // that happens to enforce it today, and the next caller of classify will not + // have read resolve.ts. Captured, a SOURCE_COMMIT would put the source box's + // (usually empty) value into the age store, under a name a template already + // refers to — the trap laundered through the one artifact nobody can read. + assertNoReservedEnvNames( + required + .filter((r) => isReservedEnvName(r.key)) + .map(({ resource, key }) => ({ resource, key })), + ); const generatedSet = new Set(generated); const plan: Disposition[] = []; const missing: Classification["missing"] = []; diff --git a/src/cli.ts b/src/cli.ts index 04505d7..5b16801 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -65,6 +65,7 @@ import { renderInventory, renderSweep, } from "./inventory.js"; +import { assertNoReservedEnvNames, reservedHits } from "./reserved.js"; import { PATH_IN_PROD_REFUSAL, desiredFromManifest, @@ -2113,6 +2114,16 @@ export function buildExecutor( await client.patch(`/${base}/${uuid}`, apiFields); }, async syncEnv(uuid, kind, env) { + // The reserved-name rule at the wire (reserved.ts). Nothing can reach here + // carrying one — resolve.ts refuses the manifest long before a diff, let + // alone an apply — and the check is here anyway, because this is the single + // function in cast that puts an env var on a Coolify resource, and the + // invariant being protected is exactly "cast never writes one". A future + // caller of buildExecutor will not have read resolve.ts; the guard it needs + // is the one standing where the write happens. + assertNoReservedEnvNames( + reservedHits(`${kind} ${uuid}`, Object.keys(env.vars)), + ); // Bulk env update is an UPSERT of listed keys, not a full replace — // verified against app/Http/Controllers/Api/{Applications,Databases, // Services}Controller.php@create_bulk_envs (coollabsio/coolify diff --git a/src/diff.ts b/src/diff.ts index 2555bb4..6e03606 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -1,4 +1,5 @@ import type { ResolvedEnv } from "./envtemplate.js"; +import { isReservedEnvName, reservedConsequence } from "./reserved.js"; export type ResourceKind = "application" | "database" | "service"; export type Desired = { @@ -63,10 +64,29 @@ export type Placement = { split: boolean; }; +// A reserved name (SOURCE_COMMIT, COOLIFY_*) found on a LIVE resource. +// +// NOT an orphan var, and the distinction is the whole point of this type. An +// orphan var is a live-only var the manifest does not declare, and its +// documented disposition is "apply never removes these; read them by eye" — +// cosmetic residue, filed under a heading that invites being read past. A +// reserved name is not residue: it is an ACTIVE SUPPRESSION of a value Coolify +// would otherwise inject (see reserved.ts), it is the difference between +// /version reporting a commit and reporting "unknown", and it is never +// cosmetic. So it comes out of that list and is reported as a finding, with the +// consequence attached. +// +// `apply never deletes` still holds, unchanged: cast reports it, a human deletes +// it in the Coolify UI. +export type ReservedVar = { kind: ResourceKind; name: string; key: string }; + export type DiffReport = { mode: "structural" | "full"; changes: Change[]; orphans: { kind: ResourceKind; name: string; uuid: string }[]; + // Findings, not drift-to-repair. Never empty in structural mode by accident: + // structural mode reads no env vars at all, so it can find none — and says so. + reserved: ReservedVar[]; placement: Placement; clean: boolean; }; @@ -92,12 +112,34 @@ function diffEnv( diffs.push({ key, state: "change", secret: v.secret }); } for (const key of Object.keys(live)) { - if (!(key in desired.vars)) + // A reserved name is deliberately NOT a remove-candidate: it is collected + // separately, as a finding (see ReservedVar). Leaving it here as well would + // report the same var twice under two headings, one of which says it is + // harmless. It also cannot be an `add`/`change`: the manifest side can never + // declare one — resolve.ts refuses the run first. + if (!(key in desired.vars) && !isReservedEnvName(key)) diffs.push({ key, state: "remove-candidate", secret: false }); } return diffs; } +// Read off the LIVE side, and off every live resource — not only the ones the +// manifest declares. A reserved name suppresses Coolify's injection on the box +// whether or not cast has ever heard of the resource carrying it, so scanning +// `changes` (which exists only for declared resources) would miss it on exactly +// the resource nobody is watching. Empty in structural mode, where no env var +// was read at all. +function reservedVars(live: Live[]): ReservedVar[] { + const found: ReservedVar[] = []; + for (const l of live) { + for (const key of Object.keys(l.env ?? {})) { + if (isReservedEnvName(key)) + found.push({ kind: l.kind, name: l.name, key }); + } + } + return found; +} + function computePlacement(live: Live[], declared?: string): Placement { const byDestination = new Map(); for (const l of live) { @@ -175,15 +217,26 @@ export function computeDiff( .filter((l) => !desired.some((d) => d.kind === l.kind && d.name === l.name)) .map((l) => ({ kind: l.kind, name: l.name, uuid: l.uuid })); const placement = computePlacement(live, opts.declaredDestination); + const reserved = reservedVars(live); return { mode, changes, orphans, + reserved, placement, // A split project is drift, and drift is not clean — the same disposition // as an orphan: reported, counted, and NOT repaired (apply moves nothing // between networks; see renderDiff). - clean: changes.length === 0 && orphans.length === 0 && !placement.split, + // + // A reserved name is not clean either, and for a stronger reason than drift: + // it is a live defect. The box it sits on is deploying green and reporting + // the wrong commit, and a `diff` that answered "clean" over it would be the + // last chance anyone had to notice. + clean: + changes.length === 0 && + orphans.length === 0 && + reserved.length === 0 && + !placement.split, }; } @@ -215,6 +268,19 @@ export function renderDiff(report: DiffReport): string { `orphan ${o.kind} ${o.name} (live, not in manifest — removal is a manual runbook act)`, ); } + // Printed as a FINDING, in its own paragraph, with the consequence attached — + // not as a one-line entry in a list of things that are fine. The failure this + // catches is green: the deploy worked, the health check passed, and this line + // is the only place anything says otherwise. It has to be readable as an + // instruction to go and delete something, because that is what it is. + for (const r of report.reserved) { + lines.push( + `FINDING: ${r.kind} ${r.name} carries env var ${r.key} — DELETE IT (Coolify UI)`, + ` ${reservedConsequence(r.key)}`, + " cast declares no such var and never will (it refuses a manifest that does),", + " and `apply` never deletes — so this one is yours to remove, by hand, in the UI.", + ); + } const { placement } = report; if (placement.split) { lines.push( @@ -269,8 +335,10 @@ export function renderDiff(report: DiffReport): string { report.clean ? "clean" : `${report.changes.length} change(s), ${report.orphans.length} orphan(s)${ - placement.split ? ", split placement" : "" - }`, + report.reserved.length > 0 + ? `, ${report.reserved.length} reserved-name FINDING(s)` + : "" + }${placement.split ? ", split placement" : ""}`, ); return lines.join("\n"); } diff --git a/src/draft.ts b/src/draft.ts index 8fa0697..9ac1952 100644 --- a/src/draft.ts +++ b/src/draft.ts @@ -2,6 +2,7 @@ import { existsSync, mkdirSync, readdirSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { stringify } from "yaml"; import { GENERATED_PLACEHOLDER } from "./capture.js"; +import { isReservedEnvName, reservedConsequence } from "./reserved.js"; import { encryptSecrets } from "./secrets.js"; // `inventory` can already SEE a whole instance (#22). This is it writing down @@ -275,7 +276,26 @@ export type DraftContext = { generatedAt: string; }; -export type Provenance = "captured" | "generated"; +// What the draft did with a name it read off the live box. +// +// captured — its value went to the store, and a ${REF} to the template. +// generated — provider-generated (see isProviderGenerated): placeheld, value +// not read into any artifact. +// suppressed — reserved by the platform (see reserved.ts): SOURCE_COMMIT, +// COOLIFY_*. Not in the template, not in the store, not +// anywhere — and named in UNCAPTURED.md, because a name cast +// declines to carry has to be said out loud rather than dropped. +// +// `suppressed` is the third one because the second was not enough. Before it, +// isProviderGenerated was the ONLY filter between a live var and a drafted +// manifest, and it recognizes SERVICE_* and datastore-connection names — nothing +// else. `SOURCE_COMMIT` splits to [SOURCE, COMMIT]: no SERVICE_ prefix, no +// datastore word, no connection word. So it was captured verbatim, with its live +// (usually EMPTY) value, and drafting a working box reproduced in the new box's +// manifest the exact var that suppresses Coolify's own injection — which the next +// `apply` would then dutifully write. The draft's whole promise is that it does +// not carry a box's traps forward. +export type Provenance = "captured" | "generated" | "suppressed"; export type DraftDisposition = { project: string; @@ -622,6 +642,10 @@ function planSecrets( const sites = byKey.get(key) ?? []; sites.push({ resource: r.name, value }); byKey.set(key, sites); + // Dispositioned (above — it gets an entry, and a line in the table), but + // never templated: a reserved name in an emitted template is the trap + // itself, copied forward. See Provenance / reserved.ts. + if (isReservedEnvName(key)) continue; usable.push([key, ""]); } if (usable.length > 0) templates.set(r.name, usable); @@ -632,12 +656,26 @@ function planSecrets( const refOf = new Map(); // `${resource}::${key}` -> ref for (const [key, sites] of byKey) { - const provenance: Provenance = isProviderGenerated(key) - ? "generated" - : "captured"; + const provenance: Provenance = isReservedEnvName(key) + ? "suppressed" + : isProviderGenerated(key) + ? "generated" + : "captured"; + if (provenance === "suppressed") { + // Said out loud, in the file that exists precisely so that what cast + // declines to carry is stated rather than dropped. The reader is being + // told two things: it is not in your draft, AND it is a live bug on the + // box you drafted from. + uncaptured.push({ + project: p.name, + setting: `env var ${key}`, + detail: `${sites.map((s) => `"${s.resource}"`).join(", ")} set ${key} on this box. It is NOT in this draft — not in a template, not in the store, and its live value was not read into any artifact. ${reservedConsequence(key)} Carrying it into the new box's manifest would reproduce that suppression there, and the first \`apply\` would write it; cast refuses a manifest that declares one. Delete it on the source box too (Coolify UI) — it is suppressing the injection there right now.`, + }); + } // A provider-generated name is placeheld everywhere it appears, so two // resources disagreeing about its value is not a conflict cast has to - // resolve — neither value is being carried. + // resolve — neither value is being carried. Same for a suppressed one, and + // more so: it is not being carried anywhere at all. const distinct = new Set(sites.map((s) => s.value)); const split = provenance === "captured" && distinct.size > 1; if (split) { @@ -668,8 +706,17 @@ function planSecrets( // THE line this whole file is bent around: a provider-generated name is // placeheld with the same literal `capture` writes, and the source box's // value is not written anywhere — not into a template, not into a store, - // not into a log. - value: provenance === "generated" ? GENERATED_PLACEHOLDER : s.value, + // not into a log. A suppressed name gets no value at all: it is not + // placeheld, because there is nothing for it to be a placeholder FOR — + // the platform supplies it, and the correct manifest says nothing. The + // empty string here never reaches an artifact (planDraft drops suppressed + // entries from the store), and it must not start to. + value: + provenance === "generated" + ? GENERATED_PLACEHOLDER + : provenance === "suppressed" + ? "" + : s.value, }); if (provenance === "generated" && !generated.includes(ref)) generated.push(ref); @@ -865,6 +912,15 @@ export function renderUncaptured( "recognize **will have been copied**. The disposition table printed at the end of", "the run is the list to read.", "", + "Names **reserved by the platform** (`SOURCE_COMMIT`, `COOLIFY_*`) were", + "**suppressed**: not written to a template, not written to the store, not carried", + "at all. Coolify injects those itself at runtime — and it *skips* its own", + "injection when the resource already carries a var of that name, so a var of that", + "name (even an EMPTY one) suppresses the platform's value, on a deploy that stays", + "green. If one is listed above, it is not merely absent from this draft: it is", + "doing that, right now, on the box this was read from. Delete it in the Coolify", + "UI.", + "", ); return lines.join("\n"); } @@ -973,8 +1029,13 @@ export function planDraft( }); } + // Suppressed names never reach the store. A `SOURCE_COMMIT` in it would be + // a name waiting for a template to reference it — and the store is the one + // artifact a reviewer does not read, because it is encrypted. const vars = Object.fromEntries( - secrets.dispositions.map((d) => [d.ref, d.value]), + secrets.dispositions + .filter((d) => d.provenance !== "suppressed") + .map((d) => [d.ref, d.value]), ); if (Object.keys(vars).length > 0) { stores.push({ @@ -1288,17 +1349,24 @@ export function renderDraftPlan( : a.project.localeCompare(b.project), )) { const note = - d.provenance === "generated" ? ` → ${GENERATED_PLACEHOLDER}` : ""; + d.provenance === "generated" + ? ` → ${GENERATED_PLACEHOLDER}` + : d.provenance === "suppressed" + ? " → NOT COPIED (Coolify injects this itself)" + : ""; lines.push( - ` ${d.ref.padEnd(width)} ${d.provenance.padEnd(9)} ${d.sites.join(", ")}${note}`, + ` ${d.ref.padEnd(width)} ${d.provenance.padEnd(10)} ${d.sites.join(", ")}${note}`, ); } const generated = plan.dispositions.filter( (d) => d.provenance === "generated", ).length; + const suppressed = plan.dispositions.filter( + (d) => d.provenance === "suppressed", + ).length; lines.push( "", - `${plan.dispositions.length} name(s): ${plan.dispositions.length - generated} captured, ${generated} placeheld as provider-generated.`, + `${plan.dispositions.length} name(s): ${plan.dispositions.length - generated - suppressed} captured, ${generated} placeheld as provider-generated${suppressed > 0 ? `, ${suppressed} suppressed (reserved by Coolify — and a live bug on the box you drafted from: see UNCAPTURED.md)` : ""}.`, "", "A placeheld name's LIVE VALUE WAS NOT READ INTO ANY FILE. A DATABASE_URL copied", "off this box points at THIS box's Postgres: a rebuilt box carrying it comes up", diff --git a/src/reserved.ts b/src/reserved.ts new file mode 100644 index 0000000..5aceaee --- /dev/null +++ b/src/reserved.ts @@ -0,0 +1,112 @@ +// Names whose meaning belongs to the PLATFORM, and which cast therefore never +// writes, never copies, and never lets a manifest declare. +// +// Coolify injects a set of values into an application's runtime environment +// itself — `SOURCE_COMMIT`, and the `COOLIFY_*` family (`COOLIFY_URL`, +// `COOLIFY_FQDN`, `COOLIFY_BRANCH`, `COOLIFY_RESOURCE_UUID`, +// `COOLIFY_CONTAINER_NAME`). It does so behind one guard, and that guard is the +// entire reason this file exists — app/Jobs/ApplicationDeploymentJob.php +// (coollabsio/coolify v4.1.2), in generate_coolify_env_variables: +// +// if ($this->application->environment_variables->where('key', 'SOURCE_COMMIT')->isEmpty()) { +// if (! is_null($this->commit)) { +// $coolify_envs->put('SOURCE_COMMIT', $this->commit); +// } else { +// $coolify_envs->put('SOURCE_COMMIT', 'unknown'); +// } +// } +// +// (v4.1.2, lines 2994-3001; the same `->isEmpty()` shape guards each COOLIFY_* +// name at 3002-3028, and again on the preview branch at 2950-2984.) +// +// Coolify SKIPS its own injection of a name when the application already carries +// an env var of that name. So an application-level `SOURCE_COMMIT` does not +// merely fail to help — it **suppresses** the value Coolify would otherwise have +// provided. An EMPTY one suppresses it exactly as completely: presence is the +// whole test, `isEmpty()` being asked of the collection of vars, never of the +// value. +// +// And it fails GREEN. The deploy succeeds, the health check passes, the +// container runs — and the only symptom is that `/version`, which reads +// `process.env.SOURCE_COMMIT` at request time, reports `unknown`. That is the +// endpoint a production cutover is gated on. (D-266.) +// +// WHY THIS IS IN CAST'S CODE AND NOT IN THE STATE FILE. `forbidden_var_patterns` +// (envtemplate.ts) is the neighbouring rule and looks like the obvious home for +// this one. It is not. That rule is POLICY — an environment's own choice about +// its own vars, which prod may set harder than staging, and which therefore +// lives in the operator's private state precisely so a product-side change +// cannot lower its own guard. This rule is a FACT ABOUT COOLIFY: true on every +// box, in every environment, for every project. There is no environment in which +// declaring `SOURCE_COMMIT` is correct, so there must be no file in which it can +// be permitted. +// +// Not to be confused with cast's OWN config vars — `COOLIFY_BASE_URL`, +// `COOLIFY_ACCESS_TOKEN`, `COOLIFY_READ_ONLY` (config.ts). Those are read out of +// the operator's local instance file and are never written to a resource. The +// namespace collides; the meaning does not. Nothing here applies to them. + +// Exactly the two shapes, and no more. Widening this set is not free: every name +// added here is a name cast will REFUSE to carry, so a wrong entry breaks a +// manifest that was right. Add one only with the guard in Coolify's source that +// justifies it. +export const RESERVED_EXACT: readonly string[] = ["SOURCE_COMMIT"]; +export const RESERVED_PREFIX = /^COOLIFY_/; + +export function isReservedEnvName(key: string): boolean { + return RESERVED_EXACT.includes(key) || RESERVED_PREFIX.test(key); +} + +// One sentence, wherever a reserved name has to be reported rather than refused +// (`diff` on a live box, `inventory --emit-draft`'s UNCAPTURED.md). Whatever the +// verb, the consequence is the same sentence — a reader who has met it once in a +// diff recognizes it in a draft. +export function reservedConsequence(key: string): string { + return `${key} is injected by Coolify itself at runtime, and Coolify SKIPS its own injection when the resource already carries a var of that name (ApplicationDeploymentJob.php, v4.1.2). A var of this name — even an EMPTY one — SUPPRESSES the platform's value. The deploy stays green and /version reports "unknown".`; +} + +export type ReservedHit = { resource: string; key: string }; + +export function reservedHits( + resource: string, + keys: Iterable, +): ReservedHit[] { + const hits: ReservedHit[] = []; + for (const key of keys) { + if (isReservedEnvName(key)) hits.push({ resource, key }); + } + return hits; +} + +export function renderReservedRefusal(hits: ReservedHit[]): string { + const width = Math.max(...hits.map((h) => h.resource.length)); + return [ + `refusing this manifest: ${hits.length} env var(s) declare a name Coolify injects itself`, + "", + ...hits.map((h) => ` ${h.resource.padEnd(width)} ${h.key}`), + "", + "Coolify injects SOURCE_COMMIT and the COOLIFY_* family into an application's", + "runtime environment itself — and it SKIPS its own injection of a name the", + "resource already carries a var of (ApplicationDeploymentJob.php, v4.1.2). A", + "declared var of that name therefore does not merely fail to help: it SUPPRESSES", + "the value Coolify would otherwise have provided.", + "", + "Presence, not value. An empty one suppresses it exactly as completely — the same", + 'rule forbidden_var_patterns already holds to, for the same reason: "off" means', + "absent, not empty.", + "", + "And it fails GREEN. The deploy succeeds, the health check passes, and the only", + 'symptom is /version reporting "unknown" — the endpoint a production cutover is', + "gated on.", + "", + "Delete the line from the env template. There is nothing to replace it with:", + "Coolify sets the value at runtime, on every deploy, from no declaration at all.", + ].join("\n"); +} + +// Every manifest read passes through here (resolve.ts), so a reserved name fails +// the run BEFORE any write — never at the wire, and never half-applied. +export function assertNoReservedEnvNames(hits: ReservedHit[]): void { + if (hits.length === 0) return; + throw new Error(renderReservedRefusal(hits)); +} diff --git a/src/resolve.ts b/src/resolve.ts index 43bb888..6586fb3 100644 --- a/src/resolve.ts +++ b/src/resolve.ts @@ -10,6 +10,11 @@ import { templateRefs, } from "./envtemplate.js"; import { loadManifest } from "./manifest.js"; +import { + type ReservedHit, + assertNoReservedEnvNames, + reservedHits, +} from "./reserved.js"; // How cast authenticated (or failed to authenticate) a clone. // @@ -219,6 +224,17 @@ export function requiredSecrets( ); } const required: RequiredSecret[] = []; + // Reserved names are checked HERE, and in manifestResources, and in + // desiredFromManifest — every function in this file that opens an env + // template, rather than once in the verb that writes. The rule is a property + // of cast, not of `apply`: a template that declares SOURCE_COMMIT is broken + // whether the verb about to run is going to write it (`apply`), store its live + // value (`capture`), or merely compare it (`diff`, `inventory`). Refusing in + // one place and reporting in another would leave `capture` writing a store for + // a manifest `apply` will refuse — a green run that guarantees a red one. See + // reserved.ts. (Reads ALL template keys, not just the ${…} refs: a bare + // `SOURCE_COMMIT=` literal suppresses the injection exactly as well.) + const reserved: ReservedHit[] = []; const collect = (resource: string, template?: string) => { if (!template) return; const file = join(checkoutDir, ".infra", "env", template); @@ -226,7 +242,9 @@ export function requiredSecrets( throw new Error( `env template missing: ${file} (referenced by ${resource})`, ); - for (const { key, ref } of templateRefs(readFileSync(file, "utf8"))) { + const text = readFileSync(file, "utf8"); + reserved.push(...reservedHits(resource, templateKeys(text))); + for (const { key, ref } of templateRefs(text)) { required.push({ ref, resource, key }); } }; @@ -236,6 +254,7 @@ export function requiredSecrets( for (const [name, svc] of Object.entries(envSpec.services ?? {})) { collect(name, svc.env_template); } + assertNoReservedEnvNames(reserved); const generated = envSpec.generated_secrets ?? []; // A generated_secrets entry naming something no template refs is dead // config — and dead config in THIS list is not merely untidy, it is @@ -282,6 +301,7 @@ export function manifestResources( `environment ${envName} not in manifest (has: ${Object.keys(manifest.environments).join(", ") || "none"})`, ); } + const reserved: ReservedHit[] = []; const keysOf = (resource: string, template?: string): string[] => { if (!template) return []; const file = join(checkoutDir, ".infra", "env", template); @@ -289,9 +309,11 @@ export function manifestResources( throw new Error( `env template missing: ${file} (referenced by ${resource})`, ); - return templateKeys(readFileSync(file, "utf8")); + const keys = templateKeys(readFileSync(file, "utf8")); + reserved.push(...reservedHits(resource, keys)); + return keys; }; - return [ + const resources = [ ...Object.entries(envSpec.applications).map(([name, app]) => ({ kind: "application" as const, name, @@ -308,6 +330,8 @@ export function manifestResources( envKeys: keysOf(name, svc.env_template), })), ]; + assertNoReservedEnvNames(reserved); + return resources; } export function desiredFromManifest( @@ -332,6 +356,7 @@ export function desiredFromManifest( string, { frequency: string; retention: number } > = {}; + const reserved: ReservedHit[] = []; const resolveEnvFile = ( name: string, template?: string, @@ -341,6 +366,7 @@ export function desiredFromManifest( if (!existsSync(file)) throw new Error(`env template missing: ${file} (referenced by ${name})`); const env = resolveTemplate(readFileSync(file, "utf8"), secrets); + reserved.push(...reservedHits(name, Object.keys(env.vars))); resolvedEnvs[name] = env; return env; }; @@ -406,5 +432,9 @@ export function desiredFromManifest( env: resolveEnvFile(name, svc.env_template), }); } + // Before the caller can diff it, and long before apply can write it: a + // resolved env that carries a reserved name is not desired state, it is a + // suppression of the platform's own value dressed up as one. See reserved.ts. + assertNoReservedEnvNames(reserved); return { desired, resolvedEnvs, backupSchedules }; } diff --git a/src/smoke.ts b/src/smoke.ts index 68c069c..45570b9 100644 --- a/src/smoke.ts +++ b/src/smoke.ts @@ -1,11 +1,29 @@ import type { CoolifyClient } from "./coolify.js"; +import { assertNoReservedEnvNames, reservedHits } from "./reserved.js"; + +// smoke writes env vars onto a REAL application — the one thing in cast that +// mutates a live resource purely to learn something. So the reserved-name rule +// binds it too, and these are the two names it may pick from. +// +// Exported, and asserted below, because the rule is about what cast writes, not +// about what one function happens to have been written to write today: an +// operator renaming a probe (`COOLIFY_SMOKE_PROBE` reads like the natural name) +// would otherwise set the very trap this rule exists to prevent, on a live app, +// and the smoke would pass while suppressing the app's SOURCE_COMMIT injection +// for as long as the probe lived — and the delete at the end restores nothing: +// Coolify only injects at deploy time. +export const SMOKE_KEEP_KEY = "INFRA_SMOKE_KEEP"; +export const SMOKE_PROBE_KEY = "INFRA_SMOKE_PROBE"; export async function smoke( client: CoolifyClient, targetAppUuid: string, ): Promise { - const KEEP_KEY = "INFRA_SMOKE_KEEP"; - const PROBE_KEY = "INFRA_SMOKE_PROBE"; + const KEEP_KEY = SMOKE_KEEP_KEY; + const PROBE_KEY = SMOKE_PROBE_KEY; + assertNoReservedEnvNames( + reservedHits(`smoke probe on ${targetAppUuid}`, [KEEP_KEY, PROBE_KEY]), + ); const envsPath = `/applications/${targetAppUuid}/envs`; type EnvVar = { key: string; diff --git a/test/reserved.test.ts b/test/reserved.test.ts new file mode 100644 index 0000000..18bb21c --- /dev/null +++ b/test/reserved.test.ts @@ -0,0 +1,351 @@ +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { classify } from "../src/capture.js"; +import { computeDiff, renderDiff } from "../src/diff.js"; +import { type DraftProject, planDraft } from "../src/draft.js"; +import { isReservedEnvName } from "../src/reserved.js"; +import { + desiredFromManifest, + manifestResources, + requiredSecrets, +} from "../src/resolve.js"; +import { SMOKE_KEEP_KEY, SMOKE_PROBE_KEY } from "../src/smoke.js"; + +// #50. Coolify injects SOURCE_COMMIT and the COOLIFY_* family itself, and SKIPS +// its own injection of a name the resource already carries a var of +// (ApplicationDeploymentJob.php, v4.1.2). So a var of that name SUPPRESSES the +// platform's value — and it fails GREEN: the deploy succeeds, the health check +// passes, and /version reports "unknown". +// +// The rule has to be true of CAST, not of one code path — every place cast +// touches an env var. This file tests all four of them together, because that +// joint property is the thing being claimed. + +describe("the rule", () => { + it("reserves the names Coolify injects itself", () => { + for (const key of [ + "SOURCE_COMMIT", + "COOLIFY_URL", + "COOLIFY_FQDN", + "COOLIFY_BRANCH", + "COOLIFY_RESOURCE_UUID", + "COOLIFY_CONTAINER_NAME", + "COOLIFY_ANYTHING_AT_ALL", + ]) { + expect(isReservedEnvName(key), key).toBe(true); + } + }); + + // The rule is exactly two shapes. A name that merely LOOKS adjacent is a + // manifest's own business — over-reaching here refuses a manifest that was + // right, which is the one way this rule can do harm. + it("reserves nothing else", () => { + for (const key of [ + "SOURCE_COMMIT_SHA", + "MY_SOURCE_COMMIT", + "COOLIFYISH", + "SERVICE_FQDN_UMAMI", + "DATABASE_URL", + "NODE_ENV", + ]) { + expect(isReservedEnvName(key), key).toBe(false); + } + }); +}); + +// --- resolve / apply: REFUSE --------------------------------------------------- + +function checkout(template: string, envName = "staging"): string { + const dir = mkdtempSync(join(tmpdir(), "infra-reserved-")); + mkdirSync(join(dir, ".infra", "env"), { recursive: true }); + writeFileSync( + join(dir, ".infra", "manifest.yaml"), + `project: widget +environments: + ${envName}: + applications: + core: + source: { repo: acme/widget, branch: main } + build: { pack: nixpacks, base_directory: / } + port: 3000 + domains: [] + env_template: core.${envName}.env.template +`, + ); + writeFileSync( + join(dir, ".infra", "env", `core.${envName}.env.template`), + template, + ); + return dir; +} + +describe("resolve / apply — a manifest that declares a reserved name is refused", () => { + it("refuses before anything is written, naming the var and the consequence", () => { + const dir = checkout("PORT=3000\nSOURCE_COMMIT=${SOURCE_COMMIT}\n"); + expect(() => + desiredFromManifest(dir, "staging", { SOURCE_COMMIT: "abc123" }), + ).toThrow(/SOURCE_COMMIT/); + try { + desiredFromManifest(dir, "staging", { SOURCE_COMMIT: "abc123" }); + } catch (e) { + const msg = String(e); + expect(msg).toMatch(/refusing/); + expect(msg).toMatch(/core/); // which resource + expect(msg).toMatch(/SUPPRESSES/); // what it does + expect(msg).toMatch(/GREEN/); // and how it fails + expect(msg).toMatch(/version/); + } + }); + + // The whole trap, in one test. An EMPTY SOURCE_COMMIT is not a var that does + // nothing — it is the var that suppresses the injection most invisibly, and it + // is the one the real box actually carried. Presence, not value: the same rule + // forbidden_var_patterns already holds to. + it("refuses an EMPTY literal — presence, not value", () => { + const dir = checkout("PORT=3000\nSOURCE_COMMIT=\n"); + expect(() => desiredFromManifest(dir, "staging", {})).toThrow( + /SOURCE_COMMIT/, + ); + }); + + it("refuses the COOLIFY_* family too", () => { + const dir = checkout("COOLIFY_URL=https://app.example.com\n"); + expect(() => desiredFromManifest(dir, "staging", {})).toThrow( + /COOLIFY_URL/, + ); + }); + + // Not just apply: every verb that reads the manifest. Refusing in one and + // reporting in another would let `capture` write a store for a manifest + // `apply` is guaranteed to refuse — a green run that promises a red one. + it("refuses on the capture path (requiredSecrets) and the inventory path (manifestResources)", () => { + const dir = checkout("SOURCE_COMMIT=${SOURCE_COMMIT}\n"); + expect(() => requiredSecrets(dir, "staging")).toThrow(/SOURCE_COMMIT/); + expect(() => manifestResources(dir, "staging")).toThrow(/SOURCE_COMMIT/); + }); + + it("leaves an ordinary manifest alone", () => { + const dir = checkout("PORT=3000\nMG=${MG}\n"); + const { desired } = desiredFromManifest(dir, "staging", { MG: "v" }); + expect(desired).toHaveLength(1); + }); +}); + +// --- capture: NEVER STORE ONE -------------------------------------------------- + +describe("capture — classify refuses a reserved name at the file", () => { + // Unreachable through the CLI (requiredSecrets refuses first) and asserted + // anyway: the invariant is "cast never carries one", not "the CLI happens to + // check first". A captured SOURCE_COMMIT would sit in the age store — the one + // artifact a reviewer cannot read. + it("refuses rather than reading the live value into the store", () => { + expect(() => + classify( + [{ ref: "SOURCE_COMMIT", resource: "core", key: "SOURCE_COMMIT" }], + [], + { core: { SOURCE_COMMIT: "" } }, + {}, + ), + ).toThrow(/SOURCE_COMMIT/); + }); +}); + +// --- draft: NEVER COPY ONE ----------------------------------------------------- + +const draftCtx = { + env: "prod", + instance: "box-b", + baseUrl: "https://coolify.example.com", + team: { id: 0, name: "Root Team" }, + server: "box-b", + recipient: "age1example", + generatedAt: "2026-07-13T00:00:00.000Z", +}; + +// The live box that set this whole thing off: a working app carrying an orphan, +// EMPTY SOURCE_COMMIT. Before #50, isProviderGenerated was the only filter on +// what a live var becomes in a drafted manifest — and SOURCE_COMMIT splits to +// [SOURCE, COMMIT]: no SERVICE_ prefix, no datastore word, no connection word. +// So it was captured verbatim, and drafting a working box reproduced the trap in +// the new box's manifest. +const suppressingBox = (): DraftProject => ({ + name: "Incubator", + coolifyEnv: "staging", + resources: [ + { + kind: "application", + name: "core", + uuid: "a1", + raw: { + git_repository: "https://github.com/heavy-duty/incubator", + git_branch: "main", + build_pack: "nixpacks", + base_directory: "/", + ports_exposes: "3000", + fqdn: "https://app.example.com", + }, + env: { + SOURCE_COMMIT: "", + COOLIFY_BRANCH: "main", + MAILGUN_KEY: "key-abc123", + }, + }, + ], + unreadable: [], + otherEnvironments: [], +}); + +describe("draft — a reserved name is suppressed, not copied", () => { + const plan = planDraft([suppressingBox()], draftCtx); + const template = plan.files.find((f) => f.path.endsWith(".env.template")); + + it("keeps it out of the emitted env template", () => { + expect(template?.content).toContain("MAILGUN_KEY="); + expect(template?.content).not.toContain("SOURCE_COMMIT"); + expect(template?.content).not.toContain("COOLIFY_BRANCH"); + }); + + it("keeps it out of the age store", () => { + const store = plan.stores[0]; + expect(Object.keys(store.vars)).toEqual(["MAILGUN_KEY"]); + }); + + it("dispositions it as `suppressed` rather than dropping it in silence", () => { + const d = plan.dispositions.find((x) => x.ref === "SOURCE_COMMIT"); + expect(d?.provenance).toBe("suppressed"); + expect(d?.sites).toEqual(["core.SOURCE_COMMIT"]); + expect( + plan.dispositions.find((x) => x.ref === "COOLIFY_BRANCH")?.provenance, + ).toBe("suppressed"); + expect( + plan.dispositions.find((x) => x.ref === "MAILGUN_KEY")?.provenance, + ).toBe("captured"); + }); + + // UNCAPTURED.md exists precisely so that what cast declines to carry is stated + // out loud rather than dropped — and this entry has to say two things: it is + // not in your draft, AND it is a live bug on the box you drafted from. + it("names it in UNCAPTURED.md, with the consequence", () => { + const uncaptured = plan.uncaptured.find( + (u) => u.setting === "env var SOURCE_COMMIT", + ); + expect(uncaptured).toBeDefined(); + expect(uncaptured?.detail).toMatch(/SUPPRESSES/); + expect(uncaptured?.detail).toMatch(/NOT in this draft/); + const md = plan.files.find((f) => f.path === "UNCAPTURED.md"); + expect(md?.content).toContain("SOURCE_COMMIT"); + }); + + // A resource whose only vars were reserved has no template to point at, and + // must not gain an env_template line for a file that does not exist. + it("emits no env template at all when every var was reserved", () => { + const box = suppressingBox(); + box.resources[0].env = { SOURCE_COMMIT: "" }; + const p = planDraft([box], draftCtx); + expect(p.files.some((f) => f.path.endsWith(".env.template"))).toBe(false); + expect(p.stores).toHaveLength(0); + const manifest = p.files.find((f) => f.path.endsWith("manifest.yaml")); + expect(manifest?.content).not.toContain("env_template"); + }); +}); + +// --- diff: A FINDING, NOT AN ORPHAN VAR ---------------------------------------- + +const desiredApp = { + kind: "application" as const, + name: "core", + fields: { build_pack: "nixpacks" }, + env: { vars: { PORT: { value: "3000", secret: false } } }, +}; +const liveApp = (env: Record) => ({ + kind: "application" as const, + name: "core", + uuid: "u1", + fields: { build_pack: "nixpacks" }, + env, +}); + +describe("diff — a reserved name on a live box is a finding", () => { + it("is promoted OUT of the remove-candidate orphan list", () => { + const r = computeDiff( + [desiredApp], + [liveApp({ PORT: "3000", SOURCE_COMMIT: "" })], + "full", + ); + // The category whose documented meaning is "apply never removes these; read + // them by eye". This is not cosmetic residue, so it must not be filed as it. + const orphanVars = r.changes.flatMap((c) => + c.envDiffs.filter((e) => e.state === "remove-candidate"), + ); + expect(orphanVars).toEqual([]); + expect(r.reserved).toEqual([ + { kind: "application", name: "core", key: "SOURCE_COMMIT" }, + ]); + }); + + it("is not clean — the box is deploying green and reporting the wrong commit", () => { + const r = computeDiff( + [desiredApp], + [liveApp({ PORT: "3000", SOURCE_COMMIT: "" })], + "full", + ); + expect(r.clean).toBe(false); + const out = renderDiff(r); + expect(out).toMatch(/FINDING/); + expect(out).toMatch(/DELETE IT/); + expect(out).toMatch(/SUPPRESSES/); + expect(out).toMatch(/reserved-name FINDING\(s\)/); + // apply never deletes — cast reports it, the human removes it in the UI. + expect(out).toMatch(/never deletes/); + }); + + // A reserved var suppresses the injection whether or not cast has ever heard + // of the resource carrying it — so the scan is over the LIVE side, not over + // the resources the manifest happens to declare. + it("finds one on an orphan resource, which no change entry covers", () => { + const r = computeDiff( + [desiredApp], + [ + liveApp({ PORT: "3000" }), + { + kind: "application" as const, + name: "nobody-declared-me", + uuid: "u2", + fields: {}, + env: { COOLIFY_URL: "https://stale.example.com" }, + }, + ], + "full", + ); + expect(r.reserved).toEqual([ + { + kind: "application", + name: "nobody-declared-me", + key: "COOLIFY_URL", + }, + ]); + expect(r.clean).toBe(false); + }); + + it("finds none in structural mode, where no env var was read at all", () => { + const r = computeDiff([desiredApp], [liveApp({})], "structural"); + expect(r.reserved).toEqual([]); + }); + + it("stays clean on a box with no reserved names", () => { + const r = computeDiff([desiredApp], [liveApp({ PORT: "3000" })], "full"); + expect(r.clean).toBe(true); + expect(renderDiff(r)).not.toMatch(/FINDING/); + }); +}); + +// --- smoke: it writes an env var too ------------------------------------------- + +describe("smoke — the probe it writes can never be a reserved name", () => { + it("picks names outside the reserved space", () => { + expect(isReservedEnvName(SMOKE_KEEP_KEY)).toBe(false); + expect(isReservedEnvName(SMOKE_PROBE_KEY)).toBe(false); + }); +});