heavy-duty/cast
7
0
Fork 0
Code Issues 4 Pull requests Projects Releases 3 Packages Wiki Activity Actions

Merge pull request #55 from claude-hdb/fix/generated-secret-guard

Browse source 
fix(apply): refuse to write the generated-secret placeholder over a live value (#47)
This commit is contained in:
Daniel Marin 2026-07-15 00:04:24 +01:00 committed by GitHub
commit 0fecfbf5d7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 527 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
docs/semantics.md
View file

@ -580,6 +580,50 @@ dangerous, because it reads like a guard standing over a name while standing
over nothing, and the likeliest cause is a typo whose real name is then
*captured* from the source box instead of placeheld.
### The placeholder is a promise, and `apply` refuses to write it over a value
The bootstrap is **two-pass**, and only the first pass is safe to repeat.
`pending-coolify-generated` means *"no real value exists yet — Coolify will make
one"*. The first `apply` sends it, Coolify creates the Postgres/Redis and
replaces it with the real URL. **From that moment the store is known-wrong**, and
a second `apply` would PATCH the placeholder back over the live, working value
and redeploy every consumer onto it — Coolify's bulk env endpoint is a plain
upsert (`create_bulk_envs`, v4.1.2: an existing key is found and its value
overwritten), so nothing on the far side stops it either.
So `diff` and `apply` both know the literal:
- `diff` gives it its own state and its own words — `secret DATABASE_URL: store
holds the generated-secret PLACEHOLDER, live holds a real value — apply would
OVERWRITE it`, plus a count in the summary line. The old report said `secret
DATABASE_URL differs`, which is what a legitimate **rotation** of the same
secret prints: the one signal there was could not be told from routine.
- `apply` **refuses** — it does not warn. Data-loss write, same fail-closed
family as the team assert and the absent-project gate. Refused before any
resource is touched, and the refusal names the key and the resource, **never
the live value**.
The rule, exactly:
| store value | live value | disposition |
| --- | --- | --- |
| placeholder | a real value | **refuse** (an already-generated secret) |
| placeholder | placeholder | proceed (nothing differs) |
| placeholder | absent | proceed (`add` — this is the first pass) |
| placeholder | *resource does not exist* | proceed (`create` — Coolify replaces it) |
| a real value | anything | proceed (an ordinary rotation) |
It is keyed on the **store's value**, not on the manifest's `generated_secrets:`
list — that list names store *refs* (`DATABASE_URL_PROD`) while an env diff is
keyed by env var *key* (`DATABASE_URL`), and the template maps one to the other.
The value is the same fact, carried to where it is needed. It is also the
stricter reading: a name dropped from `generated_secrets:` while the store still
holds the placeholder is still a write of a promise over a value.
The other half of this hole is that nothing can yet **fill** the store after the
first apply — `capture` placeholds a generated secret by design. Until it can,
the refusal is the guard and filling the store is a manual act.
**Secret hygiene**, all enforced by tests against real values:
- The plan prints **names and provenance, never values**. (The one value-shaped

41
src/apply.ts
View file

@ -1,4 +1,11 @@
import type { Change, Desired, DiffReport, ResourceKind } from "./diff.js";
import { GENERATED_PLACEHOLDER } from "./capture.js";
import {
type Change,
type Desired,
type DiffReport,
type ResourceKind,
placeholderConflicts,
} from "./diff.js";
import type { ResolvedEnv } from "./envtemplate.js";
export type Executor = {
@ -95,6 +102,38 @@ export async function applyPlan(
"apply requires a full diff (session token with read:sensitive) — refusing on a structural report",
);
}
// REFUSED, not warned about. The placeholder is a promise ("Coolify will make
// this"), never a value, and writing it over a secret Coolify has since made
// is a data-loss write: it takes DATABASE_URL away from every consumer and
// then redeploys them onto it. A warning is no guard at all here, because the
// plan line it would sit next to is indistinguishable from a routine rotation
// — this is the same fail-closed family as the team assert and the absent-
// project gate, and for the same reason: a routine command about to do
// something irreversible.
//
// Before ANY resource is touched, like the not-updatable refusal below: an
// apply that pulled the database out from under one app and only THEN refused
// on the next would be the worst of both outcomes.
//
// UPDATE-path only, by construction — computeDiff can only raise this against
// a live resource (see diffEnv). A create still sends the placeholder, which
// is correct: Coolify replaces it when it makes the resource, and that is the
// first pass of the bootstrap this guard exists to let you survive twice.
const conflicts = placeholderConflicts(report);
if (conflicts.length > 0) {
throw new Error(
[
`refusing apply: the store still holds the ${GENERATED_PLACEHOLDER} placeholder for secret(s) whose live value Coolify has already generated:`,
// The key and the resource. Never the live value — capture's rule.
...conflicts.map((c) => ` ${c.key} on ${c.kind} ${c.name}`),
"",
"Writing the store's value would overwrite the real one and break every consumer.",
"Fill the store from the live resource first (`cast capture --generated-only`, #48),",
"or, if the name is no longer provider-generated, drop it from the manifest's",
"`generated_secrets:` and capture its real value.",
].join("\n"),
);
}
// Every change is checked before the first one is acted on — that is the
// guarantee ("fails loudly, before any mutation"), and it is why this is a
// separate full scan and not a check folded into the ordered walk below. A

84
src/diff.ts
View file

@ -1,3 +1,4 @@
import { GENERATED_PLACEHOLDER } from "./capture.js";
import type { ResolvedEnv } from "./envtemplate.js";
import { isReservedEnvName, reservedConsequence } from "./reserved.js";
@ -32,7 +33,12 @@ export type FieldDiff = {
};
export type EnvDiff = {
key: string;
state: "add" | "change" | "remove-candidate";
// `placeholder-conflict` is NOT a kind of `change`, and the distinction is
// the whole point: a `change` is a value cast is entitled to write, and this
// is one it must never write. The store holds GENERATED_PLACEHOLDER — "no
// real value exists yet, Coolify will make one" — and the live resource says
// Coolify already did. See diffEnv, renderDiff and applyPlan's refusal.
state: "add" | "change" | "remove-candidate" | "placeholder-conflict";
secret: boolean;
};
export type Change = {
@ -108,8 +114,43 @@ function diffEnv(
const diffs: EnvDiff[] = [];
for (const [key, v] of Object.entries(desired.vars)) {
if (!(key in live)) diffs.push({ key, state: "add", secret: v.secret });
else if (live[key] !== v.value)
diffs.push({ key, state: "change", secret: v.secret });
else if (live[key] !== v.value) {
// The second pass of the two-pass bootstrap, which for years only ever
// ran once. The store's value for a provider-generated secret is the
// literal `pending-coolify-generated` (capture.ts); the first apply sends
// it, Coolify creates the resource and replaces it with the real URL. From
// that moment the store is KNOWN-WRONG, and every diff since has printed
// `secret DATABASE_URL differs` — word for word what a legitimate rotation
// prints — while apply stood ready to PATCH the placeholder back over the
// live, working value and redeploy. So the placeholder gets its own state,
// and apply refuses on it (#47).
//
// Keyed on the STORE VALUE, not on the manifest's `generated_secrets:`
// list. Two reasons, and the first is fatal to the alternative:
//
// - `generated_secrets:` names store REFS (`DATABASE_URL_PROD`) while an
// env diff is keyed by env var KEY (`DATABASE_URL`) — the template maps
// one to the other and they routinely differ, so matching the list
// against these keys would sail straight past the real case. The value
// carries the same fact to where it is needed: resolveTemplate copies
// the store's value in verbatim, and `secret` is true exactly when the
// RHS was a single `${REF}`.
// - It is the stricter rule. A name dropped from `generated_secrets:`
// while the store still holds the placeholder is still a data-loss
// write; the placeholder is never a value anyone meant to ship.
//
// Only the UPDATE path can reach this: diffEnv runs solely against a live
// resource. On a create the placeholder is correct — Coolify replaces it —
// and that path emits `add`, untouched. A var absent live is likewise an
// `add`, and a live value that is ALSO the placeholder never gets here at
// all (the values are equal, so there is no diff to state).
const placeheld = v.secret && v.value === GENERATED_PLACEHOLDER;
diffs.push({
key,
state: placeheld ? "placeholder-conflict" : "change",
secret: v.secret,
});
}
}
for (const key of Object.keys(live)) {
// A reserved name is deliberately NOT a remove-candidate: it is collected
@ -240,6 +281,24 @@ export function computeDiff(
};
}
// Every env var whose store value is still the generated-secret placeholder
// while the live resource holds a real one. The single reading of the report
// that both renderDiff and applyPlan use, so the warning and the refusal can
// never disagree about what counts as one.
//
// Names the key and the resource — NEVER the live value. Same rule as capture's
// disposition table: the point of the report is what to fix, not what the secret
// is, and a secret printed to a terminal is a secret in a scrollback buffer.
export function placeholderConflicts(
report: DiffReport,
): Array<{ kind: ResourceKind; name: string; key: string }> {
return report.changes.flatMap((c) =>
c.envDiffs
.filter((e) => e.state === "placeholder-conflict")
.map((e) => ({ kind: c.kind, name: c.name, key: e.key })),
);
}
export function renderDiff(report: DiffReport): string {
const lines: string[] = [];
if (report.mode === "structural") {
@ -259,6 +318,14 @@ export function renderDiff(report: DiffReport): string {
lines.push(
` env ${e.key}: live-only (orphan var — apply never removes)`,
);
// Said in words no rotation prints. `secret X differs` was the ONLY signal
// this ever had, and it is exactly what a legitimate rotation of the same
// secret looks like — an operator reading it had no way to tell the two
// apart, which is how a plan to destroy a live database reads as routine.
else if (e.state === "placeholder-conflict")
lines.push(
` secret ${e.key}: store holds the generated-secret PLACEHOLDER, live holds a real value — apply would OVERWRITE it`,
);
else if (e.secret) lines.push(` secret ${e.key} differs`);
else lines.push(` env ${e.key}: ${e.state}`);
}
@ -331,6 +398,11 @@ export function renderDiff(report: DiffReport): string {
" so Coolify picks; a server with more than one destination refuses the create outright.",
);
}
// In the tail too, not only against the var: the per-var line sits inside a
// change block that can be dozens of lines up, and the summary is the line an
// operator actually reads before typing `apply`. It says what apply will do,
// which is nothing at all.
const conflicts = placeholderConflicts(report);
lines.push(
report.clean
? "clean"
@ -338,7 +410,11 @@ export function renderDiff(report: DiffReport): string {
report.reserved.length > 0
? `, ${report.reserved.length} reserved-name FINDING(s)`
: ""
}${placement.split ? ", split placement" : ""}`,
}${placement.split ? ", split placement" : ""}${
conflicts.length > 0
? `, ${conflicts.length} generated-secret PLACEHOLDER conflict(s) — apply will REFUSE`
: ""
}`,
);
return lines.join("\n");
}

210
test/apply.test.ts
View file

@ -5,6 +5,7 @@ import {
applyHostnameOverlay,
applyPlan,
} from "../src/apply.js";
import { GENERATED_PLACEHOLDER } from "../src/capture.js";
import { type Desired, type Live, computeDiff } from "../src/diff.js";
const desired: Desired[] = [
@ -70,6 +71,215 @@ describe("applyPlan", () => {
).rejects.toThrow(/build_pack.*core-api|core-api.*build_pack/s);
expect(calls).toEqual([]);
});
// #47 — the whole product of the guard. Every assertion here is about a
// routine `cast apply` against a box that has ALREADY been applied once: the
// store still holds `pending-coolify-generated` for the secrets Coolify was
// asked to generate, and Coolify has since generated them.
describe("generated-secret placeholder", () => {
const REAL_URL = "postgres://real:hunter2@db:5432/app";
const REAL_REDIS = "redis://real:hunter2@redis:6379";
const generated: Desired[] = [
{
kind: "application",
name: "core-api",
fields: { build_pack: "nixpacks" },
env: {
vars: {
DATABASE_URL: { value: GENERATED_PLACEHOLDER, secret: true },
REDIS_URL: { value: GENERATED_PLACEHOLDER, secret: true },
PORT: { value: "3000", secret: false },
},
},
},
];
const liveApp = (env: Record<string, string>) => [
{
kind: "application" as const,
name: "core-api",
uuid: "u1",
fields: { build_pack: "nixpacks" },
env,
},
];
it("REFUSES before any mutation — no syncEnv, no redeploy, nothing touched", async () => {
const { calls, exec } = recorder();
const report = computeDiff(
generated,
liveApp({
DATABASE_URL: REAL_URL,
REDIS_URL: REAL_REDIS,
PORT: "3000",
}),
"full",
);
await expect(applyPlan(report, generated, exec)).rejects.toThrow(
/refusing apply/,
);
expect(calls).toEqual([]);
});
it("names every conflicted key and its resource", async () => {
const { exec } = recorder();
const report = computeDiff(
generated,
liveApp({ DATABASE_URL: REAL_URL, REDIS_URL: REAL_REDIS }),
"full",
);
const err = await applyPlan(report, generated, exec).catch(
(e: Error) => e.message,
);
expect(err).toContain("DATABASE_URL on application core-api");
expect(err).toContain("REDIS_URL on application core-api");
expect(err).toContain(GENERATED_PLACEHOLDER);
});
it("never prints the live value it is protecting", async () => {
const { exec } = recorder();
const report = computeDiff(
generated,
liveApp({ DATABASE_URL: REAL_URL, REDIS_URL: REAL_REDIS }),
"full",
);
const err = await applyPlan(report, generated, exec).catch(
(e: Error) => e.message,
);
expect(err).not.toContain(REAL_URL);
expect(err).not.toContain(REAL_REDIS);
expect(err).not.toContain("hunter2");
});
it("tells the operator what to do about it (#48)", async () => {
const { exec } = recorder();
const report = computeDiff(
generated,
liveApp({ DATABASE_URL: REAL_URL, REDIS_URL: REAL_REDIS }),
"full",
);
const err = await applyPlan(report, generated, exec).catch(
(e: Error) => e.message,
);
expect(err).toContain("cast capture --generated-only");
expect(err).toContain("#48");
expect(err).toContain("generated_secrets:");
});
it("refuses even when the conflict rides along with legitimate drift", async () => {
const { calls, exec } = recorder();
const withDomain: Desired[] = [
{
...generated[0],
fields: { build_pack: "nixpacks", domains: ["https://new.example"] },
},
];
const report = computeDiff(
withDomain,
liveApp({
DATABASE_URL: REAL_URL,
REDIS_URL: REAL_REDIS,
PORT: "3000",
}),
"full",
);
await expect(applyPlan(report, withDomain, exec)).rejects.toThrow(
/refusing apply/,
);
// The updatable field drift is real and would otherwise have been applied.
// The refusal is not a filter: nothing at all goes out.
expect(calls).toEqual([]);
});
it("refuses on a conflict carried by a SECOND resource, after a clean first one", async () => {
const { calls, exec } = recorder();
const two: Desired[] = [
{
kind: "application",
name: "web",
fields: { build_pack: "nixpacks" },
env: { vars: { PORT: { value: "3000", secret: false } } },
},
...generated,
];
const live = [
{
kind: "application" as const,
name: "web",
uuid: "u0",
fields: { build_pack: "static" },
env: { PORT: "3000" },
},
...liveApp({ DATABASE_URL: REAL_URL, REDIS_URL: REAL_REDIS }),
];
// `web` also carries non-updatable drift, so if the refusals were ordered
// the other way this would throw for the wrong reason — the data-loss
// write is the one an operator must be told about first.
const report = computeDiff(two, live, "full");
await expect(applyPlan(report, two, exec)).rejects.toThrow(
/refusing apply.*DATABASE_URL/s,
);
expect(calls).toEqual([]);
});
// The FIRST apply, which must keep working: the placeholder is what cast is
// supposed to send, because Coolify replaces it when it creates the resource.
it("still sends the placeholder on a create", async () => {
const { calls, exec } = recorder();
const r = await applyPlan(
computeDiff(generated, [], "full"),
generated,
exec,
);
expect(calls).toEqual([
"create core-api",
"env new-uuid",
"redeploy new-uuid",
]);
expect(r.mutated).toEqual(["core-api"]);
});
// Applied once, resources not yet generated (or generated as the placeholder
// — same thing to cast). Nothing differs, so there is nothing to refuse.
it("proceeds when the live value is the placeholder too", async () => {
const { calls, exec } = recorder();
const report = computeDiff(
generated,
liveApp({
DATABASE_URL: GENERATED_PLACEHOLDER,
REDIS_URL: GENERATED_PLACEHOLDER,
PORT: "3000",
}),
"full",
);
const r = await applyPlan(report, generated, exec);
expect(calls).toEqual([]);
expect(r.mutated).toEqual([]);
});
// A var the manifest declares and the live resource has never had: writing
// the placeholder is the only thing cast can do, and it is what the first
// apply's second pass needs.
it("proceeds when the generated var is absent live", async () => {
const { calls, exec } = recorder();
const report = computeDiff(generated, liveApp({ PORT: "3000" }), "full");
const r = await applyPlan(report, generated, exec);
expect(calls).toEqual(["env u1", "redeploy u1"]);
expect(r.mutated).toEqual(["core-api"]);
});
// The guard must not turn every secret rotation into a refusal — that is the
// failure that gets a guard disabled.
it("still applies an ordinary secret rotation", async () => {
const { calls, exec } = recorder();
const rotated: Desired[] = [
{
kind: "application",
name: "core-api",
fields: { build_pack: "nixpacks" },
env: { vars: { MAILGUN_KEY: { value: "mk-NEW", secret: true } } },
},
];
const report = computeDiff(
rotated,
liveApp({ MAILGUN_KEY: "mk-OLD" }),
"full",
);
const r = await applyPlan(report, rotated, exec);
expect(calls).toEqual(["env u1", "redeploy u1"]);
expect(r.mutated).toEqual(["core-api"]);
});
});
it("does nothing on a clean report", async () => {
const { calls, exec } = recorder();
const live = [

154
test/diff.test.ts
View file

@ -1,5 +1,6 @@
import { describe, expect, it } from "vitest";
import { computeDiff, renderDiff } from "../src/diff.js";
import { GENERATED_PLACEHOLDER } from "../src/capture.js";
import { computeDiff, placeholderConflicts, renderDiff } from "../src/diff.js";
const desiredApp = {
kind: "application" as const,
@ -90,6 +91,157 @@ describe("computeDiff", () => {
});
});
// The second pass of the two-pass bootstrap (#47). The store holds the literal
// `pending-coolify-generated` for a provider-generated secret; once the first
// apply has run, Coolify holds the real one. Every fixture here is that state.
const REAL_URL = "postgres://real:secret@db:5432/app";
const generatedApp = {
kind: "application" as const,
name: "core-api",
fields: {},
env: {
vars: {
// The env var KEY. In the real manifest the store REF behind it is
// `DATABASE_URL_PROD` — the two differ, which is why the guard is keyed
// on the store's VALUE and not on the `generated_secrets:` name list.
DATABASE_URL: { value: GENERATED_PLACEHOLDER, secret: true },
PORT: { value: "3000", secret: false },
},
},
};
const liveGenerated = (env: Record<string, string>) => [
{
kind: "application" as const,
name: "core-api",
uuid: "u1",
fields: {},
env,
},
];
describe("computeDiff generated-secret placeholder", () => {
it("flags a placeholder-in-store vs real-value-live as a conflict, not a change", () => {
const r = computeDiff(
[generatedApp],
liveGenerated({ DATABASE_URL: REAL_URL, PORT: "3000" }),
"full",
);
expect(r.changes[0].envDiffs).toEqual([
{ key: "DATABASE_URL", state: "placeholder-conflict", secret: true },
]);
expect(r.clean).toBe(false);
expect(placeholderConflicts(r)).toEqual([
{ kind: "application", name: "core-api", key: "DATABASE_URL" },
]);
});
it("is clean when the live value is the placeholder too (first apply landed, resource not created yet)", () => {
const r = computeDiff(
[generatedApp],
liveGenerated({ DATABASE_URL: GENERATED_PLACEHOLDER, PORT: "3000" }),
"full",
);
expect(r.clean).toBe(true);
expect(placeholderConflicts(r)).toEqual([]);
});
it("is a plain add — not a conflict — when the var is absent live (the FIRST apply's path)", () => {
const r = computeDiff(
[generatedApp],
liveGenerated({ PORT: "3000" }),
"full",
);
expect(r.changes[0].envDiffs).toEqual([
{ key: "DATABASE_URL", state: "add", secret: true },
]);
expect(placeholderConflicts(r)).toEqual([]);
});
it("is a plain add on a create — Coolify replaces the placeholder when it makes the resource", () => {
const r = computeDiff([generatedApp], [], "full");
expect(r.changes[0].op).toBe("create");
expect(r.changes[0].envDiffs).toContainEqual({
key: "DATABASE_URL",
state: "add",
secret: true,
});
expect(placeholderConflicts(r)).toEqual([]);
});
it("leaves an ordinary secret rotation a plain change", () => {
const r = computeDiff(
[desiredApp],
[
{
kind: "application" as const,
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: { PORT: "3000", MAILGUN_KEY: "mk-OLD" },
},
],
"full",
);
expect(r.changes[0].envDiffs).toEqual([
{ key: "MAILGUN_KEY", state: "change", secret: true },
]);
expect(placeholderConflicts(r)).toEqual([]);
});
// A non-secret template literal that happens to read `pending-coolify-generated`
// came from the template, not the store — nothing generated it, and writing it
// is what the manifest asked for.
it("does not flag a non-secret var whose literal value happens to be the placeholder", () => {
const r = computeDiff(
[
{
kind: "application" as const,
name: "core-api",
fields: {},
env: {
vars: { NOTE: { value: GENERATED_PLACEHOLDER, secret: false } },
},
},
],
liveGenerated({ NOTE: "something-else" }),
"full",
);
expect(r.changes[0].envDiffs).toEqual([
{ key: "NOTE", state: "change", secret: false },
]);
expect(placeholderConflicts(r)).toEqual([]);
});
it("is invisible to a structural diff, which reads no env at all", () => {
const r = computeDiff(
[generatedApp],
liveGenerated({ DATABASE_URL: REAL_URL }),
"structural",
);
expect(placeholderConflicts(r)).toEqual([]);
expect(r.clean).toBe(true);
});
});
describe("renderDiff generated-secret placeholder", () => {
it("says it in words no rotation prints, and never prints the live value", () => {
const out = renderDiff(
computeDiff(
[generatedApp],
liveGenerated({ DATABASE_URL: REAL_URL, PORT: "3000" }),
"full",
),
);
expect(out).toContain(
"secret DATABASE_URL: store holds the generated-secret PLACEHOLDER, live holds a real value — apply would OVERWRITE it",
);
// The old line — the one a rotation prints — must NOT be what this reports.
expect(out).not.toContain("secret DATABASE_URL differs");
// Loud in the tail as well: the summary is the line read before typing apply.
expect(out).toContain(
"1 generated-secret PLACEHOLDER conflict(s) — apply will REFUSE",
);
// Names the key, never the value — capture's rule (a secret printed to a
// terminal is a secret in a scrollback buffer).
expect(out).not.toContain(REAL_URL);
expect(out).not.toContain("real:secret");
});
});
// The destination can never be diffed the way a field is: Coolify 4.1.2 takes
// destination_uuid on write and returns destination_id on read, with nothing
// mapping between them. So it is REPORTED rather than compared — and the one