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

Merge pull request #79 from claude-hdb/fix/env-diff-real-value

Browse source 
fix(diff): compare non-secret env vars against fresh value, not stale real_value (#78)
This commit is contained in:
Daniel Marin 2026-07-16 17:43:03 +02:00 committed by GitHub
commit 6ad6259e72
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 162 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
src/cli.ts
View file

@ -51,6 +51,7 @@ import {
import {
type Change,
type Live,
type LiveEnvVar,
type ResourceKind,
computeDiff,
renderDiff,
@ -822,8 +823,11 @@ export function aliasLive<T extends { name: string }>(
});
}
// A live resource's env vars, by key. `real_value` is the decrypted one and
// needs a token with read:sensitive; `value` is what a lesser token sees.
// A live resource's env vars, by key, each carrying both forms Coolify returns:
// `value` (fresh after every write, but masked for a secret to a plain token)
// and `realValue` (the decrypted plaintext, needs a token with read:sensitive).
// The diff picks between them per var (see LiveEnvVar, diffEnv); callers that
// only need one flattened string use flattenEnv below.
//
// A 404 (a resource we just listed no longer having an envs endpoint — not
// expected in practice, but consistent with treating "gone" as "no env vars")
@ -834,13 +838,25 @@ export function aliasLive<T extends { name: string }>(
async function fetchEnv(
client: CoolifyClient,
l: Pick<Live, "kind" | "uuid">,
): Promise<Record<string, string>> {
): Promise<Record<string, LiveEnvVar>> {
const base = l.kind === "database" ? "databases" : `${l.kind}s`;
const envs = (await client.get(`/${base}/${l.uuid}/envs`).catch((err) => {
if (err instanceof HttpError && err.status === 404) return [];
throw err;
})) as Array<{ key: string; real_value?: string; value: string }>;
return Object.fromEntries(envs.map((e) => [e.key, e.real_value ?? e.value]));
return Object.fromEntries(
envs.map((e) => [e.key, { value: e.value, realValue: e.real_value }]),
);
}
// Collapse a live env map to one string per key, preferring the decrypted
// `realValue` — what capture writes into a store and what draft scaffolds from,
// where the plaintext of a secret is the point and the diff's stale-`realValue`
// hazard (#78) does not apply (nothing is compared against a manifest literal).
function flattenEnv(env: Record<string, LiveEnvVar>): Record<string, string> {
return Object.fromEntries(
Object.entries(env).map(([k, e]) => [k, e.realValue ?? e.value]),
);
}
// The databases inside ONE project+environment, each carrying the value it
@ -1643,7 +1659,7 @@ async function main(): Promise<number> {
}
const liveEnvs: LiveEnvs = {};
for (const l of envBearing) {
liveEnvs[l.name] = await fetchEnv(client, l);
liveEnvs[l.name] = flattenEnv(await fetchEnv(client, l));
}
const classification = classify(
required,
@ -1857,7 +1873,9 @@ async function main(): Promise<number> {
// Databases hold no manifest-templated env of their own — their URL is
// what the APPS reference, and that name is generated, not captured.
if (r.kind === "database") continue;
r.env = await fetchEnv(client, { kind: r.kind, uuid: r.uuid });
r.env = flattenEnv(
await fetchEnv(client, { kind: r.kind, uuid: r.uuid }),
);
}
draftProjects.push({
name: p.name,

30
src/diff.ts
View file

@ -3,6 +3,17 @@ import type { ResolvedEnv } from "./envtemplate.js";
import { isReservedEnvName, reservedConsequence } from "./reserved.js";
export type ResourceKind = "application" | "database" | "service";
// One live env var as Coolify returns it, before the per-secret flattening that
// used to happen the moment it was read. `value` is what a plain token sees:
// fresh after every write, but MASKED for a secret. `realValue` is the decrypted
// plaintext (a `read:sensitive` token); it is the only readable form of a secret,
// but for a NON-secret var it is a stored column Coolify does NOT recompute on an
// in-place PATCH — so it goes stale and reads back the pre-update value (#78).
//
// Carrying both to diffEnv (rather than collapsing to `realValue ?? value` at
// fetch time) is what lets the comparison pick the fresh side per var: `value`
// for non-secrets, `realValue ?? value` for secrets. See fetchEnv, diffEnv.
export type LiveEnvVar = { value: string; realValue?: string };
export type Desired = {
kind: ResourceKind;
name: string;
@ -14,7 +25,7 @@ export type Live = {
name: string;
uuid: string;
fields: Record<string, unknown>;
env?: Record<string, string>;
env?: Record<string, LiveEnvVar>;
// The destination (Docker network) Coolify reports this resource on.
//
// NOT in `fields`, because `fields` is the desired-vs-live comparison
@ -147,9 +158,22 @@ function eq(a: unknown, b: unknown): boolean {
return JSON.stringify(a) === JSON.stringify(b);
}
// The live value to compare a desired var against, chosen by whether the desired
// side declares it secret (#78). For a NON-secret, `value` is authoritative and
// always fresh; `realValue` is a stored column Coolify leaves stale after an
// in-place PATCH, so trusting it re-proposes an already-correct var on every
// diff. For a SECRET, `value` is masked to a plain token, so `realValue` (the
// decrypted plaintext) is the only comparable form — kept as it always was, at
// the cost of the same theoretical staleness, which a masked `value` cannot
// stand in for. `realValue` is optional (a plain token omits it); fall back to
// `value` so a secret still compares against SOMETHING rather than `undefined`.
function liveValueFor(secret: boolean, live: LiveEnvVar): string {
return secret ? (live.realValue ?? live.value) : live.value;
}
function diffEnv(
desired: ResolvedEnv,
live: Record<string, string>,
live: Record<string, LiveEnvVar>,
): EnvDiff[] {
const diffs: EnvDiff[] = [];
for (const [key, v] of Object.entries(desired.vars)) {
@ -157,7 +181,7 @@ function diffEnv(
v.derived !== undefined ? { derived: v.derived.resource } : {};
if (!(key in live))
diffs.push({ key, state: "add", secret: v.secret, ...derived });
else if (live[key] !== v.value) {
else if (liveValueFor(v.secret, 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

19
test/apply.test.ts
View file

@ -8,6 +8,13 @@ import {
import { GENERATED_PLACEHOLDER } from "../src/capture.js";
import { type Desired, type Live, computeDiff } from "../src/diff.js";
// Wrap plain live values as Coolify's {value, realValue} pairs (here the two
// agree); computeDiff reads them per LiveEnvVar. See diffEnv / #78.
const liveEnv = (
m: Record<string, string>,
): Record<string, { value: string }> =>
Object.fromEntries(Object.entries(m).map(([k, v]) => [k, { value: v }]));
const desired: Desired[] = [
{
kind: "application",
@ -63,7 +70,7 @@ describe("applyPlan", () => {
name: "core-api",
uuid: "u1",
fields: { build_pack: "static", domains: ["https://api.example.com"] },
env: { PORT: "3000" },
env: liveEnv({ PORT: "3000" }),
},
];
await expect(
@ -98,7 +105,7 @@ describe("applyPlan", () => {
name: "core-api",
uuid: "u1",
fields: { build_pack: "nixpacks" },
env,
env: liveEnv(env),
},
];
@ -201,7 +208,7 @@ describe("applyPlan", () => {
name: "web",
uuid: "u0",
fields: { build_pack: "static" },
env: { PORT: "3000" },
env: liveEnv({ PORT: "3000" }),
},
...liveApp({ DATABASE_URL: REAL_URL, REDIS_URL: REAL_REDIS }),
];
@ -291,7 +298,7 @@ describe("applyPlan", () => {
build_pack: "nixpacks",
domains: ["https://api.example.com"],
},
env: { PORT: "3000" },
env: liveEnv({ PORT: "3000" }),
},
];
const r = await applyPlan(
@ -313,7 +320,7 @@ describe("applyPlan", () => {
build_pack: "nixpacks",
domains: ["https://api.example.com"],
},
env: { PORT: "3000", LEGACY_VAR: "keep-me" },
env: liveEnv({ PORT: "3000", LEGACY_VAR: "keep-me" }),
},
];
const report = computeDiff(desired, live, "full");
@ -429,7 +436,7 @@ describe("applyPlan ordering (#45)", () => {
name: "core",
uuid: "u-core",
fields: { build_pack: "nixpacks" }, // NON_UPDATABLE drift
env: { DATABASE_URL: "postgres://x" },
env: liveEnv({ DATABASE_URL: "postgres://x" }),
},
];
await expect(

98
test/diff.test.ts
View file

@ -2,6 +2,15 @@ import { describe, expect, it } from "vitest";
import { GENERATED_PLACEHOLDER } from "../src/capture.js";
import { computeDiff, placeholderConflicts, renderDiff } from "../src/diff.js";
// Wrap plain live values as Coolify's {value, realValue} pairs. Here the two
// agree — the ordinary case, where nothing was updated in place — so only
// `value` is set; diffEnv reads it per LiveEnvVar. The stale-`realValue` case
// (#78) is exercised with explicit pairs in its own test below.
const liveEnv = (
m: Record<string, string>,
): Record<string, { value: string }> =>
Object.fromEntries(Object.entries(m).map(([k, v]) => [k, { value: v }]));
const desiredApp = {
kind: "application" as const,
name: "core-api",
@ -30,7 +39,7 @@ describe("computeDiff", () => {
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: { PORT: "3000", MAILGUN_KEY: "mk-123" },
env: liveEnv({ PORT: "3000", MAILGUN_KEY: "mk-123" }),
},
],
"full",
@ -46,7 +55,7 @@ describe("computeDiff", () => {
name: "core-api",
uuid: "u1",
fields: { build_pack: "static", domains: desiredApp.fields.domains },
env: { PORT: "3000", MAILGUN_KEY: "mk-123" },
env: liveEnv({ PORT: "3000", MAILGUN_KEY: "mk-123" }),
},
],
"full",
@ -67,7 +76,7 @@ describe("computeDiff", () => {
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: { PORT: "3000", MAILGUN_KEY: "OLD", EXTRA: "x" },
env: liveEnv({ PORT: "3000", MAILGUN_KEY: "OLD", EXTRA: "x" }),
},
];
const full = computeDiff([desiredApp], live, "full");
@ -89,6 +98,81 @@ describe("computeDiff", () => {
]);
expect(r.clean).toBe(false);
});
// #78. Coolify leaves a non-secret var's `realValue` at the pre-update value
// after an in-place PATCH, while `value` is fresh. A flip that landed and
// redeployed must read clean, not re-propose forever — compare non-secrets
// against `value`.
it("does not re-propose a non-secret var flipped in place (stale realValue)", () => {
const r = computeDiff(
[desiredApp],
[
{
kind: "application",
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: {
PORT: { value: "3000", realValue: "3000" },
// Manifest wants MAILGUN_KEY = mk-123 (a secret); realValue carries
// the plaintext and agrees, so the secret stays clean too.
MAILGUN_KEY: { value: "mk-123", realValue: "mk-123" },
},
},
],
"full",
);
expect(r.changes).toHaveLength(0);
expect(r.clean).toBe(true);
});
// The same reproduction with the flip the incubator prod cutover hit: manifest
// "true", live value already "true", but realValue still the stale "false".
it("reads a flipped-in-place flag clean even when realValue is stale", () => {
const flag = {
kind: "application" as const,
name: "core-api",
fields: {},
env: { vars: { REPORTING_ENABLED: { value: "true", secret: false } } },
};
const r = computeDiff(
[flag],
[
{
kind: "application",
name: "core-api",
uuid: "u1",
fields: {},
env: { REPORTING_ENABLED: { value: "true", realValue: "false" } },
},
],
"full",
);
expect(r.changes).toHaveLength(0);
expect(r.clean).toBe(true);
});
// The regression guard the fix must not trip: a SECRET whose `value` is masked
// to a plain token still compares via `realValue`, so a genuine rotation is
// still caught (and a masked value is never mistaken for the desired plaintext).
it("still diffs a secret via realValue when value is masked", () => {
const r = computeDiff(
[desiredApp],
[
{
kind: "application",
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: {
PORT: { value: "3000" },
MAILGUN_KEY: { value: "**********", realValue: "mk-OLD" },
},
},
],
"full",
);
expect(r.changes[0].envDiffs).toEqual([
{ key: "MAILGUN_KEY", state: "change", secret: true },
]);
});
});
// The second pass of the two-pass bootstrap (#47). The store holds the literal
@ -115,7 +199,7 @@ const liveGenerated = (env: Record<string, string>) => [
name: "core-api",
uuid: "u1",
fields: {},
env,
env: liveEnv(env),
},
];
@ -173,7 +257,7 @@ describe("computeDiff generated-secret placeholder", () => {
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: { PORT: "3000", MAILGUN_KEY: "mk-OLD" },
env: liveEnv({ PORT: "3000", MAILGUN_KEY: "mk-OLD" }),
},
],
"full",
@ -239,7 +323,7 @@ describe("derived resource refs (#60)", () => {
name: "core-api",
uuid: "u1",
fields: { ...derivedApp.fields },
env,
env: liveEnv(env),
},
];
@ -460,7 +544,7 @@ describe("renderDiff", () => {
name: "core-api",
uuid: "u1",
fields: { ...desiredApp.fields },
env: { PORT: "3000", MAILGUN_KEY: "OLD-SECRET" },
env: liveEnv({ PORT: "3000", MAILGUN_KEY: "OLD-SECRET" }),
},
];
const out = renderDiff(computeDiff([desiredApp], live, "full"));

8
test/reserved.test.ts
View file

@ -259,12 +259,18 @@ const desiredApp = {
fields: { build_pack: "nixpacks" },
env: { vars: { PORT: { value: "3000", secret: false } } },
};
// Wrap plain live values as Coolify's {value, realValue} pairs (here the two
// agree); computeDiff reads them per LiveEnvVar. See diffEnv / #78.
const liveEnv = (
m: Record<string, string>,
): Record<string, { value: string }> =>
Object.fromEntries(Object.entries(m).map(([k, v]) => [k, { value: v }]));
const liveApp = (env: Record<string, string>) => ({
kind: "application" as const,
name: "core",
uuid: "u1",
fields: { build_pack: "nixpacks" },
env,
env: liveEnv(env),
});
describe("diff — a reserved name on a live box is a finding", () => {