fix: resolve the GitHub App only when the manifest declares applications (#103)
Found live in the 2026-07-19 release drill: a manifest declaring only
databases (applications: {}) rendered its plan of two creates and then
died in preflight on "no GitHub App bound" — over a binding nothing in
the run would ever have used. A GitHub App exists to clone application
source, and cast reads it in exactly one call, the application create
(POST /applications/private-github-app); databases and services never
touch it. Resolving it unconditionally gated infra-only projects — the
databases a fleet's other projects share — behind the GitHub-App
browser-registration ceremony for no reason.
apply now resolves the App (binding lookup and uuid resolution both)
only when the desired state contains at least one application. The
executor's githubAppUuid field is typed string | null, and its single
consumer guards the null with cast's own internal error — unreachable
by construction, since a plan can only create resources the desired
state holds, but a null slipping onto the wire would otherwise surface
as a Coolify 422 about somebody else's field.
Keyed off desired rather than the plan's changes, deliberately: a
manifest that declares an application keeps the missing-binding refusal
even on a clean plan, byte-identical to before — that binding is state
the next create will need, and the operator should hear about it now,
not mid-bootstrap.
Fixes #103
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
525eac467d
commit
b2801938a4
4 changed files with 360 additions and 4 deletions
16
CHANGELOG.md
16
CHANGELOG.md
|
|
@ -7,6 +7,22 @@ actually cutting it, and this file starts there.
|
|||
|
||||
## Unreleased
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`apply` no longer demands a GitHub App for a manifest that declares no
|
||||
applications** (#103) — found live in the 2026-07-19 release drill, where a
|
||||
databases-only manifest (`applications: {}`) rendered its plan of two
|
||||
creates and then died in preflight on `no GitHub App bound`, over a binding
|
||||
nothing in the run would ever have used: a GitHub App exists to clone
|
||||
application source, cast reads it in exactly one call (the application
|
||||
create), and databases and services never touch it. That unconditional
|
||||
resolution gated infra-only projects — the databases a fleet's other
|
||||
projects share — behind the GitHub-App browser-registration ceremony for no
|
||||
reason. `apply` now resolves the App only when the desired state actually
|
||||
contains an application; a manifest that does declare one still refuses on
|
||||
a missing binding exactly as before, clean plan or not, because that
|
||||
binding is state the next create will need.
|
||||
|
||||
### Added
|
||||
|
||||
- **Tagged releases with a prebuilt dist asset, and an installer that
|
||||
|
|
|
|||
34
src/cli.ts
34
src/cli.ts
|
|
@ -1275,9 +1275,22 @@ async function runProject(
|
|||
);
|
||||
}
|
||||
const serverUuid = await ctx.client.serverUuid(ctx.binding.server);
|
||||
const githubAppUuid = await ctx.client.githubAppUuid(
|
||||
githubAppNameFor(ctx.bindings, orgRepo),
|
||||
);
|
||||
// A GitHub App is how Coolify clones application SOURCE, and cast reads it in
|
||||
// exactly one place — the application create (buildExecutor's POST
|
||||
// /applications/private-github-app). Databases and services never touch it,
|
||||
// so it is resolved only when the manifest actually declares an application.
|
||||
// Resolving it unconditionally is what killed a databases-only apply in
|
||||
// preflight (#103, found live in the 2026-07-19 release drill): the plan
|
||||
// rendered its two creates and then githubAppNameFor threw over a binding
|
||||
// nothing in the run would ever have used — gating infra-only projects
|
||||
// behind the GitHub-App browser-registration ceremony. Keyed off DESIRED
|
||||
// rather than the plan's changes, deliberately: a manifest that declares an
|
||||
// application keeps the refusal even on a clean plan, exactly as before —
|
||||
// a missing binding there is state the next create will need, and the
|
||||
// operator should hear about it now, not mid-bootstrap.
|
||||
const githubAppUuid = desired.some((d) => d.kind === "application")
|
||||
? await ctx.client.githubAppUuid(githubAppNameFor(ctx.bindings, orgRepo))
|
||||
: null;
|
||||
const exec = buildExecutor(ctx.client, {
|
||||
projectName,
|
||||
// The name the environment gets ON COOLIFY when apply creates it — so an
|
||||
|
|
@ -3155,7 +3168,10 @@ export function buildExecutor(
|
|||
projectName: string;
|
||||
envName: string;
|
||||
serverUuid: string;
|
||||
githubAppUuid: string;
|
||||
// null when the desired state declares no applications (#103): the App is
|
||||
// read by the application create alone, and runProject only resolves one
|
||||
// when there is an application for it to clone.
|
||||
githubAppUuid: string | null;
|
||||
// The three names the multi-destination 400 has to be able to say back, and
|
||||
// the only reason they are here: none of them is on the wire. A create sends
|
||||
// `serverUuid`, but the operator wrote a server NAME — and the UUID they now
|
||||
|
|
@ -3379,6 +3395,16 @@ export function buildExecutor(
|
|||
);
|
||||
const projectUuid = await projectEnv();
|
||||
if (change.kind === "application") {
|
||||
// Unreachable by construction (#103): githubAppUuid is null only
|
||||
// when the desired state holds no applications, and a plan can only
|
||||
// create resources the desired state holds. Guarded anyway — this is
|
||||
// the uuid's single consumer, and a null slipping onto the wire
|
||||
// would surface as a Coolify 422 about somebody else's field.
|
||||
if (ctx.githubAppUuid === null) {
|
||||
throw new Error(
|
||||
"internal: application create reached an executor built without a GitHub App uuid — resolution was skipped as applications-free, yet the plan creates an application",
|
||||
);
|
||||
}
|
||||
const res = (await client.post("/applications/private-github-app", {
|
||||
project_uuid: projectUuid,
|
||||
environment_name: ctx.envName,
|
||||
|
|
|
|||
213
test/github-app-cli.test.ts
Normal file
213
test/github-app-cli.test.ts
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
import { execFileSync, spawn } from "node:child_process";
|
||||
import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
|
||||
import { createServer } from "node:http";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
|
||||
// When does `apply` need a GitHub App at all? (#103, found live in the
|
||||
// 2026-07-19 release drill.)
|
||||
//
|
||||
// A GitHub App exists to clone application source, and cast reads it in exactly
|
||||
// one call — the application create. But apply used to resolve it
|
||||
// unconditionally, after the plan had already rendered: a manifest declaring
|
||||
// only databases printed its creates and then died in preflight on "no GitHub
|
||||
// App bound", over a binding nothing in the run would ever have used. That
|
||||
// gates infra-only projects (databases shared by other projects) behind the
|
||||
// GitHub-App browser-registration ceremony for no reason.
|
||||
//
|
||||
// Both directions are pinned here, end to end against a stub Coolify:
|
||||
// a databases-only manifest applies with NO github_apps binding and the stub
|
||||
// never sees a /github-apps request — and a manifest that DOES declare an
|
||||
// application still refuses on the missing binding, with the same message.
|
||||
|
||||
let recipient: string;
|
||||
let keyFile: string;
|
||||
|
||||
beforeAll(() => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "cast-age-"));
|
||||
keyFile = join(dir, "age.key");
|
||||
execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" });
|
||||
recipient = execFileSync("age-keygen", ["-y", keyFile], {
|
||||
encoding: "utf8",
|
||||
}).trim();
|
||||
});
|
||||
|
||||
type Stub = { url: string; hits: string[]; close: () => Promise<void> };
|
||||
const stubs: Stub[] = [];
|
||||
|
||||
// A box holding the project and its (empty) environment, so the plan is pure
|
||||
// creates — the drill's shape. `hits` records "METHOD path" so a test can
|
||||
// assert which routes a run touched, and which it never did.
|
||||
async function stubCoolify(): Promise<Stub> {
|
||||
const hits: string[] = [];
|
||||
const server = createServer((req, res) => {
|
||||
const path = new URL(req.url ?? "", "http://x").pathname.replace(
|
||||
"/api/v1",
|
||||
"",
|
||||
);
|
||||
hits.push(`${req.method} ${path}`);
|
||||
const json = (body: unknown) => {
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end(JSON.stringify(body));
|
||||
};
|
||||
if (path === "/teams/current") return json({ id: 0, name: "Root Team" });
|
||||
if (path === "/servers") return json([{ uuid: "s1", name: "drill-box" }]);
|
||||
if (path === "/projects" && req.method === "GET")
|
||||
return json([{ uuid: "p1", name: "drill-widget" }]);
|
||||
if (path === "/projects/p1/staging") return json({});
|
||||
if (path === "/projects/p1/environments")
|
||||
return json([{ name: "staging" }]);
|
||||
// The domain preflight's instance-wide read (only an application plan asks).
|
||||
if (path === "/applications" && req.method === "GET") return json([]);
|
||||
if (path === "/databases/redis" && req.method === "POST")
|
||||
return json({ uuid: "db-9" });
|
||||
if (path === "/deploy") return json({});
|
||||
res.writeHead(404);
|
||||
res.end("{}");
|
||||
});
|
||||
await new Promise<void>((r) => {
|
||||
server.listen(0, "127.0.0.1", r);
|
||||
});
|
||||
const stub: Stub = {
|
||||
url: `http://127.0.0.1:${(server.address() as AddressInfo).port}`,
|
||||
hits,
|
||||
close: () =>
|
||||
new Promise<void>((r) => {
|
||||
server.close(() => r());
|
||||
}),
|
||||
};
|
||||
stubs.push(stub);
|
||||
return stub;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(stubs.splice(0).map((s) => s.close()));
|
||||
});
|
||||
|
||||
// The drill's manifest: applications declared, and empty — this project IS its
|
||||
// databases.
|
||||
const DATABASES_ONLY = `project: drill-widget
|
||||
environments:
|
||||
staging:
|
||||
applications: {}
|
||||
databases:
|
||||
cache:
|
||||
type: redis
|
||||
`;
|
||||
|
||||
const WITH_APPLICATION = `project: drill-widget
|
||||
environments:
|
||||
staging:
|
||||
applications:
|
||||
web:
|
||||
source: { repo: heavy-duty/drill-widget, branch: main }
|
||||
build: { pack: nixpacks, base_directory: / }
|
||||
domains: ["http://web.example.com"]
|
||||
databases:
|
||||
cache:
|
||||
type: redis
|
||||
`;
|
||||
|
||||
// The state file the issue is about: no github_apps entry for this repo at all.
|
||||
function fixture(url: string, manifest: string) {
|
||||
const checkout = mkdtempSync(join(tmpdir(), "cast-co-"));
|
||||
mkdirSync(join(checkout, ".infra", "env"), { recursive: true });
|
||||
writeFileSync(join(checkout, ".infra", "manifest.yaml"), manifest);
|
||||
|
||||
const state = mkdtempSync(join(tmpdir(), "cast-state-"));
|
||||
mkdirSync(join(state, "secrets"));
|
||||
writeFileSync(
|
||||
join(state, ".coolify.env"),
|
||||
`COOLIFY_BASE_URL="${url}"\nCOOLIFY_ACCESS_TOKEN="t"\n`,
|
||||
);
|
||||
// No template refs any secret, but the store still has to exist and open.
|
||||
execFileSync("age", ["-r", recipient, "-o", "drill-widget.staging.env.age"], {
|
||||
input: "\n",
|
||||
cwd: join(state, "secrets"),
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
writeFileSync(
|
||||
join(state, "environments.yaml"),
|
||||
[
|
||||
"environments:",
|
||||
" staging:",
|
||||
" server: drill-box",
|
||||
" team: { id: 0, name: Root Team }",
|
||||
"github_apps: {}",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
return { checkout, state };
|
||||
}
|
||||
|
||||
function run(f: {
|
||||
checkout: string;
|
||||
state: string;
|
||||
}): Promise<{ code: number; output: string }> {
|
||||
return new Promise((resolve) => {
|
||||
const child = spawn(
|
||||
"node",
|
||||
[
|
||||
"dist/cli.js",
|
||||
"apply",
|
||||
"heavy-duty/drill-widget",
|
||||
"--env",
|
||||
"staging",
|
||||
"--path",
|
||||
f.checkout,
|
||||
"--state",
|
||||
f.state,
|
||||
],
|
||||
{
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: { ...process.env, CAST_AGE_KEY_FILE_STAGING: keyFile },
|
||||
},
|
||||
);
|
||||
let output = "";
|
||||
child.stdout.on("data", (d) => {
|
||||
output += String(d);
|
||||
});
|
||||
child.stderr.on("data", (d) => {
|
||||
output += String(d);
|
||||
});
|
||||
child.on("close", (code) => resolve({ code: code ?? 0, output }));
|
||||
});
|
||||
}
|
||||
|
||||
describe("cast apply — GitHub App resolved only for applications (#103)", () => {
|
||||
it("applies a databases-only manifest with no github_apps binding, never asking for one", async () => {
|
||||
const stub = await stubCoolify();
|
||||
const f = fixture(stub.url, DATABASES_ONLY);
|
||||
const r = await run(f);
|
||||
expect(r.code).toBe(0);
|
||||
// The apply went all the way through: the database was created and
|
||||
// redeployed, not merely planned.
|
||||
expect(r.output).toContain("applied + redeployed: cache");
|
||||
expect(stub.hits).toContain("POST /databases/redis");
|
||||
// The load-bearing absence: nothing in this run may ask Coolify for a
|
||||
// GitHub App — the resolution, not just the create, must be skipped.
|
||||
expect(stub.hits.some((h) => h.includes("/github-apps"))).toBe(false);
|
||||
expect(r.output).not.toContain("no GitHub App bound");
|
||||
});
|
||||
|
||||
it("still refuses a manifest WITH an application when the binding is missing", async () => {
|
||||
const stub = await stubCoolify();
|
||||
const f = fixture(stub.url, WITH_APPLICATION);
|
||||
const r = await run(f);
|
||||
expect(r.code).toBe(1);
|
||||
// The message githubAppNameFor has always thrown, unchanged.
|
||||
expect(r.output).toContain(
|
||||
"no GitHub App bound for heavy-duty/drill-widget",
|
||||
);
|
||||
expect(r.output).toContain('github_apps["heavy-duty/drill-widget"]');
|
||||
expect(r.output).toContain("bound repos: (none)");
|
||||
// Refused in preflight: nothing was created — not even the database the
|
||||
// manifest also declares.
|
||||
expect(stub.hits.some((h) => h.startsWith("POST /databases"))).toBe(false);
|
||||
expect(
|
||||
stub.hits.some((h) => h.includes("/applications/private-github-app")),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -498,6 +498,107 @@ describe("buildExecutor createResource (application, dockercompose)", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// githubAppUuid is null when the desired state declares no applications (#103)
|
||||
// — legal for an executor that will only ever create databases and services,
|
||||
// and unreachable for an application create by construction. The guard exists
|
||||
// so that IF the construction is ever broken, the failure is cast's own
|
||||
// sentence and not a Coolify 422 about somebody else's field.
|
||||
describe("buildExecutor createResource (no GitHub App resolved, #103)", () => {
|
||||
type Call = { method: string; path: string; body?: unknown };
|
||||
|
||||
// Records every request, so a test can assert what was NOT sent — the
|
||||
// silently-included github_app_uuid is the shape of bug this pins against.
|
||||
function recorder(handler: (path: string) => Response): {
|
||||
calls: Call[];
|
||||
fetchImpl: typeof fetch;
|
||||
} {
|
||||
const calls: Call[] = [];
|
||||
const fetchImpl = vi.fn(async (url: string | URL, init?: RequestInit) => {
|
||||
const path = new URL(String(url)).pathname;
|
||||
calls.push({
|
||||
method: init?.method ?? "GET",
|
||||
path,
|
||||
body: init?.body ? JSON.parse(String(init.body)) : undefined,
|
||||
});
|
||||
return handler(path);
|
||||
}) as unknown as typeof fetch;
|
||||
return { calls, fetchImpl };
|
||||
}
|
||||
|
||||
const ctx = {
|
||||
projectName: "drill-widget",
|
||||
envName: "staging",
|
||||
serverUuid: "srv-1",
|
||||
githubAppUuid: null,
|
||||
serverName: "drill-box",
|
||||
orgRepo: "heavy-duty/drill-widget",
|
||||
bindingEnv: "staging",
|
||||
};
|
||||
|
||||
it("creates a database without one, sending no github_app_uuid", async () => {
|
||||
const { calls, fetchImpl } = recorder((path) => {
|
||||
if (path === "/api/v1/projects")
|
||||
return new Response(
|
||||
JSON.stringify([{ uuid: "proj-1", name: "drill-widget" }]),
|
||||
{ status: 200 },
|
||||
);
|
||||
if (path === "/api/v1/projects/proj-1/environments")
|
||||
return new Response(JSON.stringify([{ name: "staging" }]), {
|
||||
status: 200,
|
||||
});
|
||||
return new Response(JSON.stringify({ uuid: "db-9" }), { status: 201 });
|
||||
});
|
||||
const exec = buildExecutor(
|
||||
new CoolifyClient("https://coolify.test", "tok", fetchImpl),
|
||||
ctx,
|
||||
);
|
||||
const uuid = await exec.createResource({
|
||||
kind: "database",
|
||||
name: "cache",
|
||||
op: "create",
|
||||
fieldDiffs: [{ field: "type", desired: "redis", updatable: false }],
|
||||
envDiffs: [],
|
||||
});
|
||||
expect(uuid).toBe("db-9");
|
||||
const create = calls.find((c) => c.path === "/api/v1/databases/redis");
|
||||
expect(create?.body).not.toHaveProperty("github_app_uuid");
|
||||
});
|
||||
|
||||
it("refuses an application create with cast's own internal error, not a wire 422", async () => {
|
||||
const { calls, fetchImpl } = recorder((path) => {
|
||||
if (path === "/api/v1/projects")
|
||||
return new Response(
|
||||
JSON.stringify([{ uuid: "proj-1", name: "drill-widget" }]),
|
||||
{ status: 200 },
|
||||
);
|
||||
if (path === "/api/v1/projects/proj-1/environments")
|
||||
return new Response(JSON.stringify([{ name: "staging" }]), {
|
||||
status: 200,
|
||||
});
|
||||
return new Response("{}", { status: 200 });
|
||||
});
|
||||
const exec = buildExecutor(
|
||||
new CoolifyClient("https://coolify.test", "tok", fetchImpl),
|
||||
ctx,
|
||||
);
|
||||
await expect(
|
||||
exec.createResource({
|
||||
kind: "application",
|
||||
name: "web",
|
||||
op: "create",
|
||||
fieldDiffs: [
|
||||
{ field: "build_pack", desired: "nixpacks", updatable: false },
|
||||
],
|
||||
envDiffs: [],
|
||||
}),
|
||||
).rejects.toThrow(/internal: application create/);
|
||||
// The refusal happened before anything went near the create route.
|
||||
expect(
|
||||
calls.some((c) => c.path === "/api/v1/applications/private-github-app"),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// Placement is create-time only, and every kind needs it: Coolify runs the same
|
||||
// destination logic in ApplicationsController, DatabasesController and
|
||||
// ServicesController, and 400s on a multi-destination server for whichever one
|
||||
|
|
|
|||
Loading…
Reference in a new issue