fix: persist the manifest conversion before the install poll can lose it
All three reviewers, independently: `createGithubApp` held the one-shot
conversion payload in memory across `awaitInstallationId` — a ~5 minute
poll — and `persistCredentials` ran only inside `registerGithubApp`. A
timeout, a dropped network or a Ctrl-C during that wait destroyed a
private key and client secret GitHub never re-shows, and left the App
orphaned on GitHub. The timeout message then claimed the credentials
were "already there" under `<state>/github-apps/`, which was false on
exactly the path that printed it.
The payload now goes to disk the instant the exchange returns, complete
but for the installation id — the one field GitHub will answer again as
often as it is asked. It is written as `installation_id: null` and
backfilled on success; `writeCredentialsRecord` allows precisely that
one transition and refuses every other difference, so nothing
irreplaceable is ever overwritten silently. The timeout path now names
the two files it wrote and prints the `register` command that finishes
the job, and says not to re-run `create`.
claude-bot's addition: persisting post-conversion could still throw in
`writeExclusive` against a stale `<name>.pem`, losing the fresh key just
the same — and that refusal's remedy ("pass --force and re-run") would
mean minting a second App. So the collision is pre-flighted before the
browser flow starts, when nothing exists and nothing can be lost. The
post-conversion persist now only ever meets a clean slot or an exact
match, and `writeExclusive`'s wording stays honest for `register`.
grok #2: re-running `register` to re-check a failed repo-visibility
assertion used to re-POST the key and the App first. Coolify does not
de-dupe by name — `GithubController@create` validates
`'name' => 'required|string|max:255'` with no `unique` rule and calls a
plain `GithubApp::create()`, and the vendored OpenAPI documents no
conflict response — so following that advice created a second Source
every time. Both verbs now read `GET /github-apps` first and verify an
existing record of that name instead of creating another; a name held by
a different App, or already duplicated, is a hard error. An unreadable
list warns and proceeds rather than blocking a bootstrap command.
grok #3: `name` becomes `<name>.pem`/`<name>.json`, so separators, dot
references, empties and control characters are rejected where the name
is resolved and again where it becomes a filename.
grok #4: every GitHub request now sends `User-Agent: cast/<version>`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
a9805d31bd
commit
5a1ec74e04
6 changed files with 923 additions and 126 deletions
14
CHANGELOG.md
14
CHANGELOG.md
|
|
@ -103,10 +103,22 @@ actually cutting it, and this file starts there.
|
||||||
`--webhook-secret` is optional: a webhook-inactive App is the correct shape
|
`--webhook-secret` is optional: a webhook-inactive App is the correct shape
|
||||||
for a tailnet-only Coolify, and nobody has to invent a placeholder.
|
for a tailnet-only Coolify, and nobody has to invent a placeholder.
|
||||||
|
|
||||||
|
The one-shot secrets are written the instant GitHub yields them, *before*
|
||||||
|
`create` waits ~5 minutes for you to install the App — so a timeout, a
|
||||||
|
dropped connection or a `Ctrl-C` during that wait cannot destroy a private
|
||||||
|
key GitHub will never re-show. Until the install lands the record carries
|
||||||
|
`"installation_id": null`, the single field GitHub will answer again, and
|
||||||
|
it is backfilled on success. A name collision is refused *before* the
|
||||||
|
browser flow starts, when no App exists yet and nothing can be lost.
|
||||||
|
|
||||||
Both verbs end at the step that matters most: `GET
|
Both verbs end at the step that matters most: `GET
|
||||||
/github-apps/{id}/repositories`, asserting the repo is actually reachable.
|
/github-apps/{id}/repositories`, asserting the repo is actually reachable.
|
||||||
Until now a misconfigured App failed silently and surfaced hours later, in
|
Until now a misconfigured App failed silently and surfaced hours later, in
|
||||||
a different command, as an unresolvable source at `cast apply` time.
|
a different command, as an unresolvable source at `cast apply` time. When
|
||||||
|
that check fails its advice is to re-run `register` — which now re-verifies
|
||||||
|
the existing Coolify Source instead of registering a second one, because
|
||||||
|
Coolify does not enforce unique Source names (`GithubController@create`
|
||||||
|
validates `name` without `unique` and calls a plain `GithubApp::create`).
|
||||||
|
|
||||||
No new dependencies — `node:http` serves the callback, `node:crypto`'s
|
No new dependencies — `node:http` serves the callback, `node:crypto`'s
|
||||||
`createSign("RSA-SHA256")` mints the App JWT that recovers the installation
|
`createSign("RSA-SHA256")` mints the App JWT that recovers the installation
|
||||||
|
|
|
||||||
32
README.md
32
README.md
|
|
@ -254,17 +254,30 @@ What `create` does:
|
||||||
which checks the CSRF `state` and shuts down.
|
which checks the CSRF `state` and shuts down.
|
||||||
5. Exchanges the code. **This response is the only moment GitHub ever hands over
|
5. Exchanges the code. **This response is the only moment GitHub ever hands over
|
||||||
the private key, the client secret and the webhook secret together.**
|
the private key, the client secret and the webhook secret together.**
|
||||||
6. Prints (and tries to open) the install URL; you pick the repository.
|
6. **Writes all three to disk immediately**, before waiting on anything —
|
||||||
7. Recovers the installation id by minting an RS256 JWT with the App's own key —
|
see [Where the credentials land](#where-the-credentials-land). Everything
|
||||||
|
after this point can fail for ordinary reasons (a slow install screen, a
|
||||||
|
dropped network, `Ctrl-C`), and none of those may cost you a key GitHub will
|
||||||
|
not reissue.
|
||||||
|
7. Prints (and tries to open) the install URL; you pick the repository.
|
||||||
|
8. Recovers the installation id by minting an RS256 JWT with the App's own key —
|
||||||
never from the `installation_id` GitHub appends to a redirect, which GitHub
|
never from the `installation_id` GitHub appends to a redirect, which GitHub
|
||||||
documents as a spoofable hint.
|
documents as a spoofable hint — then fills it into the record from step 6.
|
||||||
8. Uploads the key to Coolify and creates the App record.
|
9. Uploads the key to Coolify and creates the App record — unless a Source of
|
||||||
9. **Asks Coolify which repositories the App can actually see, and fails if
|
that name already exists, in which case it verifies that one rather than
|
||||||
|
registering a second (Coolify does not enforce unique Source names).
|
||||||
|
10. **Asks Coolify which repositories the App can actually see, and fails if
|
||||||
`<org>/<repo>` is not among them.** This is the step that matters most:
|
`<org>/<repo>` is not among them.** This is the step that matters most:
|
||||||
without it a misconfigured App fails silently and surfaces hours later, in a
|
without it a misconfigured App fails silently and surfaces hours later, in a
|
||||||
different command, as an unresolvable source at `cast apply` time.
|
different command, as an unresolvable source at `cast apply` time.
|
||||||
|
|
||||||
`register` is the same command from step 8 onwards, for an App you already hold —
|
If the install never lands, `create` stops at step 8 and tells you the exact
|
||||||
|
`register` command that finishes the job against the files from step 6. Nothing
|
||||||
|
is lost and nothing has to be recreated — in particular, do **not** re-run
|
||||||
|
`create`, which would mint a second App. For that same reason `create` refuses
|
||||||
|
up front, before the browser flow, when `<name>.pem` already exists.
|
||||||
|
|
||||||
|
`register` is the same command from step 9 onwards, for an App you already hold —
|
||||||
one made by hand, or a disaster-recovery restore from a stored PEM:
|
one made by hand, or a disaster-recovery restore from a stored PEM:
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
|
|
@ -290,6 +303,13 @@ Into the state directory you point cast at — cast itself stores nothing:
|
||||||
└── <name>.json # 0600, app id, installation id, client id + secret, webhook secret
|
└── <name>.json # 0600, app id, installation id, client id + secret, webhook secret
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Both are written the instant GitHub yields them, which is *before* `create`
|
||||||
|
waits for you to install the App. Until the install lands, `<name>.json` carries
|
||||||
|
`"installation_id": null` — that is the one field GitHub will answer again as
|
||||||
|
often as it is asked, and it is filled in on success. Re-running against an
|
||||||
|
existing file is idempotent on identical content and a **refusal** otherwise;
|
||||||
|
`--force` is the deliberate escape hatch for a stale half-run.
|
||||||
|
|
||||||
All three secrets, because GitHub shows them once and `register` needs the client
|
All three secrets, because GitHub shows them once and `register` needs the client
|
||||||
secret to be re-runnable at all — a disaster-recovery restore that is missing it
|
secret to be re-runnable at all — a disaster-recovery restore that is missing it
|
||||||
is not a restore. They are written **plaintext at 0600**, not into `secrets/`:
|
is not a restore. They are written **plaintext at 0600**, not into `secrets/`:
|
||||||
|
|
|
||||||
|
|
@ -196,6 +196,11 @@ github-app (the credential Coolify clones private repos with):
|
||||||
serves a one-shot page on 127.0.0.1, your browser session authenticates
|
serves a one-shot page on 127.0.0.1, your browser session authenticates
|
||||||
the form, and the conversion response hands over the private key, the
|
the form, and the conversion response hands over the private key, the
|
||||||
client secret and the webhook secret in one body. Nothing is transcribed.
|
client secret and the webhook secret in one body. Nothing is transcribed.
|
||||||
|
All three are written to disk the instant they arrive — BEFORE the wait
|
||||||
|
for you to install the App — so a timeout or a Ctrl-C during that wait
|
||||||
|
cannot lose a key GitHub shows exactly once. If the install never lands,
|
||||||
|
cast prints the \`register\` command that finishes the job; do not re-run
|
||||||
|
\`create\`, which would mint a second App.
|
||||||
register adopts credentials you already hold: an App created by hand, or a
|
register adopts credentials you already hold: an App created by hand, or a
|
||||||
disaster-recovery restore from a stored PEM. The client secret is read
|
disaster-recovery restore from a stored PEM. The client secret is read
|
||||||
from STDIN (never argv); --webhook-secret is optional, because a
|
from STDIN (never argv); --webhook-secret is optional, because a
|
||||||
|
|
@ -207,7 +212,9 @@ github-app (the credential Coolify clones private repos with):
|
||||||
--force overwrite an existing PEM/credentials file under <state>/github-apps/.
|
--force overwrite an existing PEM/credentials file under <state>/github-apps/.
|
||||||
Both verbs end by asking Coolify which repositories the App can actually see
|
Both verbs end by asking Coolify which repositories the App can actually see
|
||||||
and failing if <org>/<repo> is not among them — the check that turns a silent
|
and failing if <org>/<repo> is not among them — the check that turns a silent
|
||||||
misconfiguration into an error next to the thing that caused it.
|
misconfiguration into an error next to the thing that caused it. Re-running
|
||||||
|
\`register\` after that failure RE-VERIFIES an existing Coolify Source of the
|
||||||
|
same name rather than registering a second one.
|
||||||
|
|
||||||
capture (adopt a hand-built instance into the age secret store):
|
capture (adopt a hand-built instance into the age secret store):
|
||||||
--generated <NAME> force NAME to the \`pending-coolify-generated\` placeholder,
|
--generated <NAME> force NAME to the \`pending-coolify-generated\` placeholder,
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
|
||||||
import { createServer } from "node:http";
|
import { createServer } from "node:http";
|
||||||
import type { AddressInfo } from "node:net";
|
import type { AddressInfo } from "node:net";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
import { parseDocument } from "yaml";
|
import { parseDocument } from "yaml";
|
||||||
import type { Bindings } from "./bindings.js";
|
import type { Bindings } from "./bindings.js";
|
||||||
import type { CoolifyClient } from "./coolify.js";
|
import type { CoolifyClient } from "./coolify.js";
|
||||||
|
|
@ -76,16 +77,58 @@ export type AppCredentials = {
|
||||||
privateKeyPem: string;
|
privateKeyPem: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// What `create` has in hand the instant the conversion returns, and what it
|
||||||
|
// persists BEFORE going anywhere near the install poll. It is an AppCredentials
|
||||||
|
// missing exactly one field, and that field is the only one recoverable later:
|
||||||
|
// GitHub will re-answer "which installation" forever, and will never re-show
|
||||||
|
// the private key or the client secret.
|
||||||
|
export type PendingAppCredentials = Omit<AppCredentials, "installationId"> & {
|
||||||
|
installationId?: number;
|
||||||
|
};
|
||||||
|
|
||||||
export type RegisterResult = {
|
export type RegisterResult = {
|
||||||
// Coolify's OWN integer id for the App record, not GitHub's app id. It is
|
// Coolify's OWN integer id for the App record, not GitHub's app id. It is
|
||||||
// what GET /github-apps/{id}/repositories takes.
|
// what GET /github-apps/{id}/repositories takes.
|
||||||
coolifyAppId: number;
|
coolifyAppId: number;
|
||||||
keyUuid: string;
|
// null when the App was already registered under this name and cast verified
|
||||||
|
// the existing record instead of creating a second one.
|
||||||
|
keyUuid: string | null;
|
||||||
repositories: string[];
|
repositories: string[];
|
||||||
pemPath: string;
|
pemPath: string;
|
||||||
secretsPath: string;
|
secretsPath: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// grok #4: GitHub asks every client to identify itself, and an absent
|
||||||
|
// User-Agent is a documented cause of 403s that look like nothing else.
|
||||||
|
// Resolved from package.json the same way `cast --version` does, and never
|
||||||
|
// fatal — a User-Agent is not worth failing a bootstrap over.
|
||||||
|
let userAgentCache: string | undefined;
|
||||||
|
export function githubUserAgent(): string {
|
||||||
|
if (userAgentCache !== undefined) return userAgentCache;
|
||||||
|
let version = "unknown";
|
||||||
|
try {
|
||||||
|
const pkgPath = fileURLToPath(new URL("../package.json", import.meta.url));
|
||||||
|
const raw: unknown = JSON.parse(readFileSync(pkgPath, "utf8")).version;
|
||||||
|
if (typeof raw === "string") version = raw;
|
||||||
|
} catch {
|
||||||
|
// A missing or unreadable package.json means an odd install tree, not a
|
||||||
|
// reason to refuse to talk to GitHub.
|
||||||
|
}
|
||||||
|
userAgentCache = `cast/${version}`;
|
||||||
|
return userAgentCache;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Every GitHub request in this module goes out with these.
|
||||||
|
function githubHeaders(
|
||||||
|
extra: Record<string, string> = {},
|
||||||
|
): Record<string, string> {
|
||||||
|
return {
|
||||||
|
Accept: "application/vnd.github+json",
|
||||||
|
"User-Agent": githubUserAgent(),
|
||||||
|
...extra,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Step 2 — the name comes from state, not from a flag
|
// Step 2 — the name comes from state, not from a flag
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
@ -106,6 +149,7 @@ export function resolveAppName(opts: {
|
||||||
nameFlag?: string;
|
nameFlag?: string;
|
||||||
}): { name: string; seed: boolean } {
|
}): { name: string; seed: boolean } {
|
||||||
const repoShort = opts.orgRepo.split("/")[1] ?? opts.orgRepo;
|
const repoShort = opts.orgRepo.split("/")[1] ?? opts.orgRepo;
|
||||||
|
if (opts.nameFlag !== undefined) assertUsableAppName(opts.nameFlag);
|
||||||
const bound =
|
const bound =
|
||||||
opts.bindings.github_apps[opts.orgRepo] ??
|
opts.bindings.github_apps[opts.orgRepo] ??
|
||||||
opts.bindings.github_apps[repoShort];
|
opts.bindings.github_apps[repoShort];
|
||||||
|
|
@ -125,6 +169,9 @@ export function resolveAppName(opts: {
|
||||||
].join("\n"),
|
].join("\n"),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
// Applies to a value that came from state too: environments.yaml is
|
||||||
|
// hand-edited, and `github_apps` entries become filenames just the same.
|
||||||
|
assertUsableAppName(bound);
|
||||||
return { name: bound, seed: false };
|
return { name: bound, seed: false };
|
||||||
}
|
}
|
||||||
if (opts.nameFlag === undefined) {
|
if (opts.nameFlag === undefined) {
|
||||||
|
|
@ -364,10 +411,7 @@ export async function convertManifestCode(
|
||||||
`https://api.github.com/app-manifests/${encodeURIComponent(code)}/conversions`,
|
`https://api.github.com/app-manifests/${encodeURIComponent(code)}/conversions`,
|
||||||
{
|
{
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: {
|
headers: githubHeaders({ "X-GitHub-Api-Version": "2022-11-28" }),
|
||||||
Accept: "application/vnd.github+json",
|
|
||||||
"X-GitHub-Api-Version": "2022-11-28",
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
|
|
@ -509,11 +553,10 @@ export async function findInstallationId(opts: {
|
||||||
? `/users/${encodeURIComponent(opts.owner)}/installation`
|
? `/users/${encodeURIComponent(opts.owner)}/installation`
|
||||||
: `/orgs/${encodeURIComponent(opts.owner)}/installation`;
|
: `/orgs/${encodeURIComponent(opts.owner)}/installation`;
|
||||||
const res = await (opts.fetchImpl ?? fetch)(`https://api.github.com${path}`, {
|
const res = await (opts.fetchImpl ?? fetch)(`https://api.github.com${path}`, {
|
||||||
headers: {
|
headers: githubHeaders({
|
||||||
Accept: "application/vnd.github+json",
|
|
||||||
Authorization: `Bearer ${opts.jwt}`,
|
Authorization: `Bearer ${opts.jwt}`,
|
||||||
"X-GitHub-Api-Version": "2022-11-28",
|
"X-GitHub-Api-Version": "2022-11-28",
|
||||||
},
|
}),
|
||||||
});
|
});
|
||||||
if (res.status === 404) return undefined;
|
if (res.status === 404) return undefined;
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
|
|
@ -528,6 +571,15 @@ export async function findInstallationId(opts: {
|
||||||
return raw.id;
|
return raw.id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Distinguishable so `create` can attach the remedy it alone can write — the
|
||||||
|
// real paths it just persisted to — without string-matching a message.
|
||||||
|
export class InstallationNeverArrivedError extends Error {
|
||||||
|
constructor(message: string) {
|
||||||
|
super(message);
|
||||||
|
this.name = "InstallationNeverArrivedError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Poll while the operator clicks through the install screen in a browser.
|
// Poll while the operator clicks through the install screen in a browser.
|
||||||
export async function awaitInstallationId(opts: {
|
export async function awaitInstallationId(opts: {
|
||||||
owner: string;
|
owner: string;
|
||||||
|
|
@ -565,18 +617,19 @@ export async function awaitInstallationId(opts: {
|
||||||
}
|
}
|
||||||
await sleep(intervalMs);
|
await sleep(intervalMs);
|
||||||
}
|
}
|
||||||
throw new Error(
|
// Only the FACT belongs here. This function does not know where — or
|
||||||
|
// whether — anything was persisted, and the previous version of this message
|
||||||
|
// asserted that credentials were "already there" on exactly the path where
|
||||||
|
// they were not (all three reviewers, #124). The caller that owns the files
|
||||||
|
// owns the remedy: see createGithubApp.
|
||||||
|
throw new InstallationNeverArrivedError(
|
||||||
[
|
[
|
||||||
`the App was never installed on ${opts.owner}`,
|
`the App was never installed on ${opts.owner}`,
|
||||||
"",
|
"",
|
||||||
`cast polled GET /${opts.ownerType === "User" ? "users" : "orgs"}/${opts.owner}/installation`,
|
`cast polled GET /${opts.ownerType === "User" ? "users" : "orgs"}/${opts.owner}/installation`,
|
||||||
`for ${Math.round((attempts * intervalMs) / 1000)}s and it stayed 404.`,
|
`for ${Math.round((attempts * intervalMs) / 1000)}s and it stayed 404.`,
|
||||||
"",
|
"",
|
||||||
"The App exists on GitHub — only the install step is missing. Open the",
|
"The App exists on GitHub — only the install step is missing.",
|
||||||
"install URL printed above, choose the target repository, and then re-run",
|
|
||||||
"`cast github-app register` with the credentials cast saved under",
|
|
||||||
"<state>/github-apps/ (the private key and client secret are already there;",
|
|
||||||
"nothing has to be recreated).",
|
|
||||||
].join("\n"),
|
].join("\n"),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -606,14 +659,88 @@ export function githubAppDir(stateDir: string): string {
|
||||||
return join(stateDir, "github-apps");
|
return join(stateDir, "github-apps");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// grok #3: `name` becomes a path segment (`<name>.pem`, `<name>.json`). It is
|
||||||
|
// operator-controlled and therefore not a security boundary — but a typo with a
|
||||||
|
// slash in it silently nests credentials under a subdirectory nobody will think
|
||||||
|
// to look in, and `..` walks out of the state directory entirely. Neither is
|
||||||
|
// worth diagnosing later, so both are refused here, at the one point where the
|
||||||
|
// name becomes a filename.
|
||||||
|
export function assertUsableAppName(name: string): void {
|
||||||
|
const bad =
|
||||||
|
name === ""
|
||||||
|
? "is empty"
|
||||||
|
: name === "." || name === ".."
|
||||||
|
? "is a directory reference"
|
||||||
|
: /[/\\]/.test(name)
|
||||||
|
? "contains a path separator"
|
||||||
|
: name.includes("..")
|
||||||
|
? "contains `..`"
|
||||||
|
: name.startsWith(".")
|
||||||
|
? "starts with a dot"
|
||||||
|
: // biome-ignore lint/suspicious/noControlCharactersInRegex: rejecting them is the point
|
||||||
|
/[\x00-\x1f]/.test(name)
|
||||||
|
? "contains a control character"
|
||||||
|
: undefined;
|
||||||
|
if (bad === undefined) return;
|
||||||
|
throw new Error(
|
||||||
|
[
|
||||||
|
`github app name ${JSON.stringify(name)} ${bad}`,
|
||||||
|
"",
|
||||||
|
"The name is used verbatim as a filename under <state>/github-apps/ and as",
|
||||||
|
"the Coolify Source label that environments.yaml binds. Use a plain name",
|
||||||
|
"like `hdb-coolify-prod`.",
|
||||||
|
].join("\n"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Refuse a name collision BEFORE the browser flow, when nothing has been
|
||||||
|
// created and nothing can be lost (claude-bot, #124).
|
||||||
|
//
|
||||||
|
// On the `create` path a stale `<name>.pem` is a trap: the freshly minted App's
|
||||||
|
// key differs from it by construction, so the post-conversion persist would hit
|
||||||
|
// writeExclusive's refusal holding the one and only copy of a key GitHub has
|
||||||
|
// already stopped showing — and that refusal's remedy ("pass --force and
|
||||||
|
// re-run") would mean minting a SECOND App. Checked here, the answer costs
|
||||||
|
// nothing: no App exists yet.
|
||||||
|
export function preflightCredentialSlot(opts: {
|
||||||
|
stateDir: string;
|
||||||
|
name: string;
|
||||||
|
force?: boolean;
|
||||||
|
}): void {
|
||||||
|
assertUsableAppName(opts.name);
|
||||||
|
if (opts.force === true) return;
|
||||||
|
const dir = githubAppDir(opts.stateDir);
|
||||||
|
const occupied = [`${opts.name}.pem`, `${opts.name}.json`]
|
||||||
|
.map((f) => join(dir, f))
|
||||||
|
.filter((p) => existsSync(p));
|
||||||
|
if (occupied.length === 0) return;
|
||||||
|
throw new Error(
|
||||||
|
[
|
||||||
|
`${opts.name} already has credentials on disk`,
|
||||||
|
"",
|
||||||
|
...occupied.map((p) => ` ${p}`),
|
||||||
|
"",
|
||||||
|
"`create` mints a NEW App, whose private key cannot match the one already",
|
||||||
|
"saved here — so this is checked now, before the browser flow, rather than",
|
||||||
|
"after GitHub has handed over a key that would have nowhere to go.",
|
||||||
|
"",
|
||||||
|
"If those files are the App you want, you do not need `create`: install it",
|
||||||
|
"on the repository and run `cast github-app register` against them.",
|
||||||
|
"If they are a stale half-run whose App you have since deleted, move them",
|
||||||
|
"aside or pass --force.",
|
||||||
|
].join("\n"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export function persistCredentials(opts: {
|
export function persistCredentials(opts: {
|
||||||
stateDir: string;
|
stateDir: string;
|
||||||
name: string;
|
name: string;
|
||||||
creds: AppCredentials;
|
creds: PendingAppCredentials;
|
||||||
org: string;
|
org: string;
|
||||||
orgRepo: string;
|
orgRepo: string;
|
||||||
force?: boolean;
|
force?: boolean;
|
||||||
}): { pemPath: string; secretsPath: string } {
|
}): { pemPath: string; secretsPath: string } {
|
||||||
|
assertUsableAppName(opts.name);
|
||||||
const dir = githubAppDir(opts.stateDir);
|
const dir = githubAppDir(opts.stateDir);
|
||||||
mkdirSync(dir, { recursive: true, mode: 0o700 });
|
mkdirSync(dir, { recursive: true, mode: 0o700 });
|
||||||
const ignore = join(dir, ".gitignore");
|
const ignore = join(dir, ".gitignore");
|
||||||
|
|
@ -639,34 +766,83 @@ export function persistCredentials(opts: {
|
||||||
const pemPath = join(dir, `${opts.name}.pem`);
|
const pemPath = join(dir, `${opts.name}.pem`);
|
||||||
const secretsPath = join(dir, `${opts.name}.json`);
|
const secretsPath = join(dir, `${opts.name}.json`);
|
||||||
writeExclusive(pemPath, opts.creds.privateKeyPem, opts.force === true);
|
writeExclusive(pemPath, opts.creds.privateKeyPem, opts.force === true);
|
||||||
writeExclusive(
|
// `installation_id: null` is the honest representation of a record written
|
||||||
secretsPath,
|
// between the conversion and the install: everything GitHub shows once is
|
||||||
`${JSON.stringify(
|
// here, and the one missing field is the one GitHub will answer again.
|
||||||
{
|
const record = {
|
||||||
name: opts.name,
|
name: opts.name,
|
||||||
org: opts.org,
|
org: opts.org,
|
||||||
repo: opts.orgRepo,
|
repo: opts.orgRepo,
|
||||||
app_id: opts.creds.appId,
|
app_id: opts.creds.appId,
|
||||||
installation_id: opts.creds.installationId,
|
installation_id: opts.creds.installationId ?? null,
|
||||||
client_id: opts.creds.clientId,
|
client_id: opts.creds.clientId,
|
||||||
client_secret: opts.creds.clientSecret,
|
client_secret: opts.creds.clientSecret,
|
||||||
webhook_secret: opts.creds.webhookSecret,
|
webhook_secret: opts.creds.webhookSecret,
|
||||||
private_key_file: `${opts.name}.pem`,
|
private_key_file: `${opts.name}.pem`,
|
||||||
},
|
};
|
||||||
null,
|
writeCredentialsRecord(secretsPath, record, opts.force === true);
|
||||||
2,
|
|
||||||
)}\n`,
|
|
||||||
opts.force === true,
|
|
||||||
);
|
|
||||||
return { pemPath, secretsPath };
|
return { pemPath, secretsPath };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Same refusal as writeExclusive, with ONE transition carved out: a record
|
||||||
|
// whose only difference from the incoming one is that its `installation_id` was
|
||||||
|
// null. That is the backfill `create` performs once the install lands, and it
|
||||||
|
// is not the loss writeExclusive exists to prevent — nothing irreplaceable
|
||||||
|
// changes. Anything else still refuses.
|
||||||
|
function writeCredentialsRecord(
|
||||||
|
path: string,
|
||||||
|
record: Record<string, unknown>,
|
||||||
|
force: boolean,
|
||||||
|
): void {
|
||||||
|
const next = `${JSON.stringify(record, null, 2)}\n`;
|
||||||
|
if (existsSync(path) && !force) {
|
||||||
|
const raw = readFileSync(path, "utf8");
|
||||||
|
if (raw === next) return;
|
||||||
|
if (!isInstallationBackfill(raw, record)) {
|
||||||
|
throw refusalToOverwrite(path);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
writeFileSync(path, next, { mode: 0o600 });
|
||||||
|
}
|
||||||
|
|
||||||
|
function isInstallationBackfill(
|
||||||
|
existing: string,
|
||||||
|
next: Record<string, unknown>,
|
||||||
|
): boolean {
|
||||||
|
let prev: Record<string, unknown>;
|
||||||
|
try {
|
||||||
|
prev = JSON.parse(existing) as Record<string, unknown>;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (prev === null || typeof prev !== "object") return false;
|
||||||
|
// Only ever fills a null in; never overwrites an id with a different one.
|
||||||
|
if (prev.installation_id !== null) return false;
|
||||||
|
if (typeof next.installation_id !== "number") return false;
|
||||||
|
const keys = new Set([...Object.keys(prev), ...Object.keys(next)]);
|
||||||
|
for (const key of keys) {
|
||||||
|
if (key === "installation_id") continue;
|
||||||
|
if (prev[key] !== next[key]) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
// Idempotent when the content matches, a refusal when it does not. Overwriting
|
// Idempotent when the content matches, a refusal when it does not. Overwriting
|
||||||
// a DIFFERENT credential silently is how the one copy of a private key is lost.
|
// a DIFFERENT credential silently is how the one copy of a private key is lost.
|
||||||
function writeExclusive(path: string, content: string, force: boolean): void {
|
function writeExclusive(path: string, content: string, force: boolean): void {
|
||||||
if (existsSync(path) && !force) {
|
if (existsSync(path) && !force) {
|
||||||
if (readFileSync(path, "utf8") === content) return;
|
if (readFileSync(path, "utf8") === content) return;
|
||||||
throw new Error(
|
throw refusalToOverwrite(path);
|
||||||
|
}
|
||||||
|
writeFileSync(path, content, { mode: 0o600 });
|
||||||
|
}
|
||||||
|
|
||||||
|
// The remedy here is honest for `register`, where re-running is cheap and
|
||||||
|
// nothing is minted. `create` can no longer reach this refusal: it is
|
||||||
|
// pre-flighted before the browser flow (preflightCredentialSlot), so by the
|
||||||
|
// time a create-path persist runs, the slot is either clean or an exact match.
|
||||||
|
function refusalToOverwrite(path: string): Error {
|
||||||
|
return new Error(
|
||||||
[
|
[
|
||||||
`refusing to overwrite ${path}`,
|
`refusing to overwrite ${path}`,
|
||||||
"",
|
"",
|
||||||
|
|
@ -678,8 +854,6 @@ function writeExclusive(path: string, content: string, force: boolean): void {
|
||||||
"previous `create` attempt whose App you have since deleted).",
|
"previous `create` attempt whose App you have since deleted).",
|
||||||
].join("\n"),
|
].join("\n"),
|
||||||
);
|
);
|
||||||
}
|
|
||||||
writeFileSync(path, content, { mode: 0o600 });
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
@ -722,6 +896,48 @@ export async function readAppRepositories(
|
||||||
return names;
|
return names;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// grok #2: the repo-visibility failure tells the operator to re-run `register`
|
||||||
|
// to re-check — and re-running used to re-POST the key and the App first. That
|
||||||
|
// is only harmless if Coolify de-dupes by name, and it does not. Coolify's own
|
||||||
|
// GithubController@create validates `'name' => 'required|string|max:255'` —
|
||||||
|
// no `unique` rule — and then calls a plain `GithubApp::create($payload)`. The
|
||||||
|
// vendored OpenAPI agrees by omission: the create response documents 201/400/
|
||||||
|
// 401/422 and no conflict at all. So a second `register` under the same name
|
||||||
|
// yields a second Source, and the remedy printed by a failed post-condition
|
||||||
|
// quietly multiplies the thing it is asking the operator to fix.
|
||||||
|
//
|
||||||
|
// Fixed by looking first. This turns "re-run register to re-check" into what
|
||||||
|
// its own wording already promised — a re-check — and leaves the create path
|
||||||
|
// unchanged, since a clean instance has nothing to find.
|
||||||
|
export type ExistingApp = { id: number; appId: number | undefined };
|
||||||
|
|
||||||
|
// `undefined` means "cast could not read the list", which is NOT the same as
|
||||||
|
// "there is nothing there" — see the caller for why that distinction does not
|
||||||
|
// become a refusal here.
|
||||||
|
export async function findRegisteredApp(
|
||||||
|
client: CoolifyClient,
|
||||||
|
name: string,
|
||||||
|
): Promise<ExistingApp[] | undefined> {
|
||||||
|
let raw: unknown;
|
||||||
|
try {
|
||||||
|
raw = await client.get("/github-apps");
|
||||||
|
} catch {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
if (!Array.isArray(raw)) return undefined;
|
||||||
|
const found: ExistingApp[] = [];
|
||||||
|
for (const item of raw) {
|
||||||
|
if (typeof item !== "object" || item === null) continue;
|
||||||
|
const row = item as Record<string, unknown>;
|
||||||
|
if (row.name !== name || typeof row.id !== "number") continue;
|
||||||
|
found.push({
|
||||||
|
id: row.id,
|
||||||
|
appId: typeof row.app_id === "number" ? row.app_id : undefined,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return found;
|
||||||
|
}
|
||||||
|
|
||||||
// Register the App with Coolify and PROVE it works.
|
// Register the App with Coolify and PROVE it works.
|
||||||
//
|
//
|
||||||
// The two POSTs are the script's, unchanged in effect. The GET is the step that
|
// The two POSTs are the script's, unchanged in effect. The GET is the step that
|
||||||
|
|
@ -755,6 +971,121 @@ export async function registerGithubApp(opts: {
|
||||||
log(`private key → ${pemPath}`);
|
log(`private key → ${pemPath}`);
|
||||||
log(`credentials → ${secretsPath}`);
|
log(`credentials → ${secretsPath}`);
|
||||||
|
|
||||||
|
// Look before creating (grok #2). Both verbs do this, so `create`'s
|
||||||
|
// fall-through remains an identity of behaviour: the same call sequence,
|
||||||
|
// in the same order, whichever verb produced the credentials.
|
||||||
|
const existing = await findRegisteredApp(opts.client, opts.name);
|
||||||
|
if (existing === undefined) {
|
||||||
|
// Deliberately a warning and not a refusal. An unreadable list leaves cast
|
||||||
|
// exactly where it was before this check existed — it might create a
|
||||||
|
// duplicate — whereas refusing would block the bootstrap command outright
|
||||||
|
// on an instance whose list endpoint is restricted. The failure mode of
|
||||||
|
// proceeding is a spare Source the operator can delete; the failure mode of
|
||||||
|
// refusing is no Source at all. Said out loud rather than assumed away.
|
||||||
|
log(
|
||||||
|
"warning: could not list existing Coolify Sources; cannot check whether",
|
||||||
|
);
|
||||||
|
log(` ${opts.name} is already registered. Proceeding to create.`);
|
||||||
|
}
|
||||||
|
if (existing !== undefined && existing.length > 1) {
|
||||||
|
throw new Error(
|
||||||
|
[
|
||||||
|
`Coolify already has ${existing.length} GitHub App records named ${opts.name}`,
|
||||||
|
"",
|
||||||
|
` coolify ids: ${existing.map((e) => e.id).join(", ")}`,
|
||||||
|
"",
|
||||||
|
"Coolify does not enforce unique Source names, so cast cannot tell which",
|
||||||
|
"of these `cast apply` would resolve. Delete the duplicates from Coolify's",
|
||||||
|
"Sources page, leaving the one that is correctly installed, then re-run.",
|
||||||
|
].join("\n"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
const reuse = existing?.[0];
|
||||||
|
if (
|
||||||
|
reuse !== undefined &&
|
||||||
|
reuse.appId !== undefined &&
|
||||||
|
reuse.appId !== opts.creds.appId
|
||||||
|
) {
|
||||||
|
throw new Error(
|
||||||
|
[
|
||||||
|
`Coolify already has a GitHub App named ${opts.name}, and it is a DIFFERENT App`,
|
||||||
|
"",
|
||||||
|
` registered: github app id ${reuse.appId} (coolify id ${reuse.id})`,
|
||||||
|
` supplied: github app id ${opts.creds.appId}`,
|
||||||
|
"",
|
||||||
|
"Registering these credentials would leave two Sources with one name and",
|
||||||
|
"no way for `cast apply` to tell them apart. Either delete the old record",
|
||||||
|
"from Coolify's Sources page, or bind this App to a different name in",
|
||||||
|
"environments.yaml.",
|
||||||
|
].join("\n"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let coolifyAppId: number;
|
||||||
|
let keyUuid: string | null = null;
|
||||||
|
if (reuse !== undefined) {
|
||||||
|
// The verify-only path. Nothing is POSTed, so a failed post-condition can
|
||||||
|
// be re-checked as many times as the operator needs.
|
||||||
|
coolifyAppId = reuse.id;
|
||||||
|
log(
|
||||||
|
`already registered as ${opts.name} (coolify id ${coolifyAppId}) — verifying, not re-creating`,
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
({ coolifyAppId, keyUuid } = await createCoolifyApp(opts, log));
|
||||||
|
}
|
||||||
|
|
||||||
|
const repositories = await readAppRepositories(opts.client, coolifyAppId);
|
||||||
|
if (repositories === undefined) {
|
||||||
|
throw new Error(
|
||||||
|
[
|
||||||
|
`cannot verify that ${opts.name} can reach ${opts.orgRepo}`,
|
||||||
|
"",
|
||||||
|
`GET /github-apps/${coolifyAppId}/repositories did not return a repository`,
|
||||||
|
"list cast can read. The App IS registered — this is a failed check, not a",
|
||||||
|
"failed registration — but the check is the point: an App that cannot see",
|
||||||
|
"the repo fails at `cast apply` time instead, hours later and somewhere",
|
||||||
|
"else.",
|
||||||
|
"",
|
||||||
|
"Open Coolify's Sources page and confirm the App lists the repository, or",
|
||||||
|
"re-run this verification with `cast github-app register` once the install",
|
||||||
|
"is fixed. Re-running re-checks the existing record; it does not create a",
|
||||||
|
"second one.",
|
||||||
|
].join("\n"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (!repositories.includes(opts.orgRepo)) {
|
||||||
|
throw new Error(
|
||||||
|
[
|
||||||
|
`${opts.name} is registered but cannot see ${opts.orgRepo}`,
|
||||||
|
"",
|
||||||
|
` can see: ${repositories.join(", ") || "(no repositories at all)"}`,
|
||||||
|
"",
|
||||||
|
"The App exists on GitHub and in Coolify; it is INSTALLED on the wrong",
|
||||||
|
"repositories (or on none). Open",
|
||||||
|
" https://github.com/settings/installations",
|
||||||
|
"or the org's Settings → GitHub Apps, grant the App access to",
|
||||||
|
`${opts.orgRepo}, and re-run \`cast github-app register\` to re-check.`,
|
||||||
|
"That re-check reuses the record above rather than registering a second.",
|
||||||
|
"",
|
||||||
|
"Left unfixed this surfaces at `cast apply` time as an unresolvable source.",
|
||||||
|
].join("\n"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
log(`verified: ${opts.name} can clone ${opts.orgRepo} ✓`);
|
||||||
|
|
||||||
|
return { coolifyAppId, keyUuid, repositories, pemPath, secretsPath };
|
||||||
|
}
|
||||||
|
|
||||||
|
// The two POSTs, unchanged in effect from the script's.
|
||||||
|
async function createCoolifyApp(
|
||||||
|
opts: {
|
||||||
|
client: CoolifyClient;
|
||||||
|
name: string;
|
||||||
|
org: string;
|
||||||
|
creds: AppCredentials;
|
||||||
|
},
|
||||||
|
log: (line: string) => void,
|
||||||
|
): Promise<{ coolifyAppId: number; keyUuid: string }> {
|
||||||
const key = (await opts.client.post("/security/keys", {
|
const key = (await opts.client.post("/security/keys", {
|
||||||
name: `${opts.name}-key`,
|
name: `${opts.name}-key`,
|
||||||
private_key: opts.creds.privateKeyPem,
|
private_key: opts.creds.privateKeyPem,
|
||||||
|
|
@ -790,45 +1121,7 @@ export async function registerGithubApp(opts: {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
log(`registered as ${opts.name} (coolify id ${coolifyAppId})`);
|
log(`registered as ${opts.name} (coolify id ${coolifyAppId})`);
|
||||||
|
return { coolifyAppId, keyUuid };
|
||||||
const repositories = await readAppRepositories(opts.client, coolifyAppId);
|
|
||||||
if (repositories === undefined) {
|
|
||||||
throw new Error(
|
|
||||||
[
|
|
||||||
`cannot verify that ${opts.name} can reach ${opts.orgRepo}`,
|
|
||||||
"",
|
|
||||||
`GET /github-apps/${coolifyAppId}/repositories did not return a repository`,
|
|
||||||
"list cast can read. The App IS registered — this is a failed check, not a",
|
|
||||||
"failed registration — but the check is the point: an App that cannot see",
|
|
||||||
"the repo fails at `cast apply` time instead, hours later and somewhere",
|
|
||||||
"else.",
|
|
||||||
"",
|
|
||||||
"Open Coolify's Sources page and confirm the App lists the repository, or",
|
|
||||||
"re-run this verification with `cast github-app register` once the install",
|
|
||||||
"is fixed.",
|
|
||||||
].join("\n"),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
if (!repositories.includes(opts.orgRepo)) {
|
|
||||||
throw new Error(
|
|
||||||
[
|
|
||||||
`${opts.name} is registered but cannot see ${opts.orgRepo}`,
|
|
||||||
"",
|
|
||||||
` can see: ${repositories.join(", ") || "(no repositories at all)"}`,
|
|
||||||
"",
|
|
||||||
"The App exists on GitHub and in Coolify; it is INSTALLED on the wrong",
|
|
||||||
"repositories (or on none). Open",
|
|
||||||
" https://github.com/settings/installations",
|
|
||||||
"or the org's Settings → GitHub Apps, grant the App access to",
|
|
||||||
`${opts.orgRepo}, and re-run \`cast github-app register\` to re-check.`,
|
|
||||||
"",
|
|
||||||
"Left unfixed this surfaces at `cast apply` time as an unresolvable source.",
|
|
||||||
].join("\n"),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
log(`verified: ${opts.name} can clone ${opts.orgRepo} ✓`);
|
|
||||||
|
|
||||||
return { coolifyAppId, keyUuid, repositories, pemPath, secretsPath };
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
@ -883,7 +1176,7 @@ export async function detectOwnerType(
|
||||||
try {
|
try {
|
||||||
const res = await fetchImpl(
|
const res = await fetchImpl(
|
||||||
`https://api.github.com/users/${encodeURIComponent(owner)}`,
|
`https://api.github.com/users/${encodeURIComponent(owner)}`,
|
||||||
{ headers: { Accept: "application/vnd.github+json" } },
|
{ headers: githubHeaders() },
|
||||||
);
|
);
|
||||||
if (!res.ok) return undefined;
|
if (!res.ok) return undefined;
|
||||||
const raw = (await res.json()) as Record<string, unknown>;
|
const raw = (await res.json()) as Record<string, unknown>;
|
||||||
|
|
@ -950,6 +1243,15 @@ export async function createGithubApp(opts: {
|
||||||
const log = deps.log ?? ((line: string) => console.log(line));
|
const log = deps.log ?? ((line: string) => console.log(line));
|
||||||
const org = opts.orgRepo.split("/")[0] ?? opts.orgRepo;
|
const org = opts.orgRepo.split("/")[0] ?? opts.orgRepo;
|
||||||
|
|
||||||
|
// Before ANY of it — before the browser, before GitHub mints anything. A
|
||||||
|
// name collision discovered here costs nothing; discovered after the
|
||||||
|
// conversion it costs the private key of an App that now exists.
|
||||||
|
preflightCredentialSlot({
|
||||||
|
stateDir: opts.stateDir,
|
||||||
|
name: opts.name,
|
||||||
|
force: opts.force,
|
||||||
|
});
|
||||||
|
|
||||||
const ownerType =
|
const ownerType =
|
||||||
opts.ownerType ??
|
opts.ownerType ??
|
||||||
(await detectOwnerType(org, deps.fetchImpl)) ??
|
(await detectOwnerType(org, deps.fetchImpl)) ??
|
||||||
|
|
@ -1017,6 +1319,44 @@ export async function createGithubApp(opts: {
|
||||||
`created GitHub App: ${conversion.slug} (github app id ${conversion.id})`,
|
`created GitHub App: ${conversion.slug} (github app id ${conversion.id})`,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// PERSIST NOW — the blocker all three reviewers raised on #124.
|
||||||
|
//
|
||||||
|
// The conversion response is the only moment GitHub ever yields the private
|
||||||
|
// key, the client secret and the webhook secret. Everything after this line
|
||||||
|
// can fail for ordinary reasons and for a long time: the install poll runs
|
||||||
|
// ~5 minutes, the operator can wander off, the network can drop, Ctrl-C is
|
||||||
|
// one keystroke. Holding the one-shot payload in memory across all of that
|
||||||
|
// and only writing it inside registerGithubApp meant any of those events
|
||||||
|
// destroyed a credential GitHub will not reissue — and left the App itself
|
||||||
|
// orphaned on GitHub, needing manual deletion.
|
||||||
|
//
|
||||||
|
// So the record goes to disk here, complete but for the installation id,
|
||||||
|
// which is the ONE field GitHub will answer again as often as asked. It is
|
||||||
|
// backfilled below once the install lands.
|
||||||
|
const webhookSecret = conversion.webhookSecret ?? generateWebhookSecret();
|
||||||
|
if (conversion.webhookSecret === null) {
|
||||||
|
log(
|
||||||
|
"github returned no webhook secret; generated one (webhook is inactive)",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
const pending: PendingAppCredentials = {
|
||||||
|
appId: conversion.id,
|
||||||
|
clientId: conversion.clientId,
|
||||||
|
clientSecret: conversion.clientSecret,
|
||||||
|
webhookSecret,
|
||||||
|
privateKeyPem: conversion.pem,
|
||||||
|
};
|
||||||
|
const saved = persistCredentials({
|
||||||
|
stateDir: opts.stateDir,
|
||||||
|
name: opts.name,
|
||||||
|
creds: pending,
|
||||||
|
org: conversion.ownerLogin,
|
||||||
|
orgRepo: opts.orgRepo,
|
||||||
|
force: opts.force,
|
||||||
|
});
|
||||||
|
log(`private key → ${saved.pemPath}`);
|
||||||
|
log(`credentials → ${saved.secretsPath} (installation id pending)`);
|
||||||
|
|
||||||
// Step 6 — install it. Always print the URL; never assume an opener.
|
// Step 6 — install it. Always print the URL; never assume an opener.
|
||||||
const installUrl = `https://github.com/apps/${conversion.slug}/installations/new`;
|
const installUrl = `https://github.com/apps/${conversion.slug}/installations/new`;
|
||||||
log("");
|
log("");
|
||||||
|
|
@ -1027,7 +1367,9 @@ export async function createGithubApp(opts: {
|
||||||
|
|
||||||
// Step 7 — recover the installation id from the App's own key, never from a
|
// Step 7 — recover the installation id from the App's own key, never from a
|
||||||
// redirect parameter.
|
// redirect parameter.
|
||||||
const installationId = await awaitInstallationId({
|
let installationId: number;
|
||||||
|
try {
|
||||||
|
installationId = await awaitInstallationId({
|
||||||
owner: conversion.ownerLogin,
|
owner: conversion.ownerLogin,
|
||||||
ownerType: conversion.ownerType,
|
ownerType: conversion.ownerType,
|
||||||
privateKeyPem: conversion.pem,
|
privateKeyPem: conversion.pem,
|
||||||
|
|
@ -1039,16 +1381,44 @@ export async function createGithubApp(opts: {
|
||||||
intervalMs: deps.installIntervalMs,
|
intervalMs: deps.installIntervalMs,
|
||||||
log,
|
log,
|
||||||
});
|
});
|
||||||
log(`installation id ${installationId} (recovered via the App JWT) ✓`);
|
} catch (err) {
|
||||||
|
// Now the "nothing has to be recreated" claim is TRUE, and it can name the
|
||||||
const webhookSecret = conversion.webhookSecret ?? generateWebhookSecret();
|
// actual files rather than a directory shape. Attached here because this is
|
||||||
if (conversion.webhookSecret === null) {
|
// the only scope that knows where the persist above landed.
|
||||||
log(
|
if (err instanceof InstallationNeverArrivedError) {
|
||||||
"github returned no webhook secret; generated one (webhook is inactive)",
|
throw new Error(
|
||||||
|
[
|
||||||
|
err.message,
|
||||||
|
"",
|
||||||
|
"Nothing is lost. cast saved everything GitHub shows only once, before",
|
||||||
|
"it started waiting:",
|
||||||
|
"",
|
||||||
|
` ${saved.pemPath}`,
|
||||||
|
` ${saved.secretsPath}`,
|
||||||
|
"",
|
||||||
|
"Install the App from the URL above, then finish with:",
|
||||||
|
"",
|
||||||
|
` cast github-app register ${opts.orgRepo} --env <env> \\`,
|
||||||
|
` --app-id ${conversion.id} --installation-id <id> \\`,
|
||||||
|
` --client-id ${conversion.clientId} \\`,
|
||||||
|
` --private-key ${saved.pemPath} --client-secret-stdin`,
|
||||||
|
"",
|
||||||
|
`The client secret and webhook secret are in ${saved.secretsPath};`,
|
||||||
|
"the installation id is on the install's own URL, or read it from",
|
||||||
|
"https://github.com/settings/installations.",
|
||||||
|
"",
|
||||||
|
"Do NOT re-run `create`: the App already exists on GitHub, and creating",
|
||||||
|
"a second one is the thing this message exists to prevent.",
|
||||||
|
].join("\n"),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
log(`installation id ${installationId} (recovered via the App JWT) ✓`);
|
||||||
|
|
||||||
// Steps 8 + 9 — the fall-through. Identical to what `register` calls.
|
// Steps 8 + 9 — the fall-through. Identical to what `register` calls; the
|
||||||
|
// persist inside it backfills the installation id onto the record written
|
||||||
|
// above rather than writing a second one.
|
||||||
return registerGithubApp({
|
return registerGithubApp({
|
||||||
client: opts.client,
|
client: opts.client,
|
||||||
name: opts.name,
|
name: opts.name,
|
||||||
|
|
@ -1057,13 +1427,6 @@ export async function createGithubApp(opts: {
|
||||||
stateDir: opts.stateDir,
|
stateDir: opts.stateDir,
|
||||||
force: opts.force,
|
force: opts.force,
|
||||||
log,
|
log,
|
||||||
creds: {
|
creds: { ...pending, installationId },
|
||||||
appId: conversion.id,
|
|
||||||
installationId,
|
|
||||||
clientId: conversion.clientId,
|
|
||||||
clientSecret: conversion.clientSecret,
|
|
||||||
webhookSecret,
|
|
||||||
privateKeyPem: conversion.pem,
|
|
||||||
},
|
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -56,6 +56,10 @@ async function stubCoolify(opts: { repositories: unknown }): Promise<Stub> {
|
||||||
if (path === "/security/keys") return json({ uuid: "key-uuid-1" });
|
if (path === "/security/keys") return json({ uuid: "key-uuid-1" });
|
||||||
if (path === "/github-apps" && req.method === "POST")
|
if (path === "/github-apps" && req.method === "POST")
|
||||||
return json({ id: 7, uuid: "app-uuid" });
|
return json({ id: 7, uuid: "app-uuid" });
|
||||||
|
// A clean instance: nothing registered under this name yet, so register
|
||||||
|
// goes on to create. (The list read is how it avoids a duplicate Source
|
||||||
|
// on a re-run — Coolify does not enforce unique names.)
|
||||||
|
if (path === "/github-apps" && req.method === "GET") return json([]);
|
||||||
if (path === "/github-apps/7/repositories")
|
if (path === "/github-apps/7/repositories")
|
||||||
return json({ repositories: opts.repositories });
|
return json({ repositories: opts.repositories });
|
||||||
res.writeHead(404);
|
res.writeHead(404);
|
||||||
|
|
|
||||||
|
|
@ -13,16 +13,20 @@ import { loadBindings } from "../src/bindings.js";
|
||||||
import { CoolifyClient } from "../src/coolify.js";
|
import { CoolifyClient } from "../src/coolify.js";
|
||||||
import {
|
import {
|
||||||
type AppCredentials,
|
type AppCredentials,
|
||||||
|
type PendingAppCredentials,
|
||||||
awaitInstallationId,
|
awaitInstallationId,
|
||||||
buildManifest,
|
buildManifest,
|
||||||
convertManifestCode,
|
convertManifestCode,
|
||||||
createGithubApp,
|
createGithubApp,
|
||||||
detectOwnerType,
|
detectOwnerType,
|
||||||
findInstallationId,
|
findInstallationId,
|
||||||
|
findRegisteredApp,
|
||||||
|
githubUserAgent,
|
||||||
manifestFormPage,
|
manifestFormPage,
|
||||||
mintAppJwt,
|
mintAppJwt,
|
||||||
newAppFormAction,
|
newAppFormAction,
|
||||||
persistCredentials,
|
persistCredentials,
|
||||||
|
preflightCredentialSlot,
|
||||||
preflightOrgAdmin,
|
preflightOrgAdmin,
|
||||||
readAppRepositories,
|
readAppRepositories,
|
||||||
registerGithubApp,
|
registerGithubApp,
|
||||||
|
|
@ -658,6 +662,7 @@ describe("registering with Coolify, and the post-condition that matters", () =>
|
||||||
|
|
||||||
it("uploads the key, creates the App, and PROVES it can reach the repo", async () => {
|
it("uploads the key, creates the App, and PROVES it can reach the repo", async () => {
|
||||||
const c = coolify({
|
const c = coolify({
|
||||||
|
"GET /github-apps": ok([{ id: 4, name: "something-else" }]),
|
||||||
"POST /security/keys": ok({ uuid: "key-uuid-1" }),
|
"POST /security/keys": ok({ uuid: "key-uuid-1" }),
|
||||||
"POST /github-apps": ok({ id: 7, uuid: "app-uuid" }),
|
"POST /github-apps": ok({ id: 7, uuid: "app-uuid" }),
|
||||||
"GET /github-apps/7/repositories": ok({
|
"GET /github-apps/7/repositories": ok({
|
||||||
|
|
@ -678,6 +683,7 @@ describe("registering with Coolify, and the post-condition that matters", () =>
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(c.hits).toEqual([
|
expect(c.hits).toEqual([
|
||||||
|
"GET /github-apps",
|
||||||
"POST /security/keys",
|
"POST /security/keys",
|
||||||
"POST /github-apps",
|
"POST /github-apps",
|
||||||
"GET /github-apps/7/repositories",
|
"GET /github-apps/7/repositories",
|
||||||
|
|
@ -787,6 +793,214 @@ describe("registering with Coolify, and the post-condition that matters", () =>
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// grok #2, and the reason it is a real bug rather than a hypothetical: Coolify
|
||||||
|
// does not enforce unique Source names. Its GithubController@create validates
|
||||||
|
// `'name' => 'required|string|max:255'` — no `unique` — then calls a plain
|
||||||
|
// `GithubApp::create()`. So the old "re-run register to re-check" advice
|
||||||
|
// created a second Source every time it was followed.
|
||||||
|
describe("re-running `register` re-verifies instead of registering twice", () => {
|
||||||
|
const ok = (payload: unknown) => () => [200, payload] as [number, unknown];
|
||||||
|
|
||||||
|
it("reuses an existing Source of the same name and POSTs NOTHING", async () => {
|
||||||
|
const c = coolify({
|
||||||
|
"GET /github-apps": ok([
|
||||||
|
{ id: 4, name: "other-app", app_id: 1 },
|
||||||
|
{ id: 7, name: "hdb-coolify-prod", app_id: 12345 },
|
||||||
|
]),
|
||||||
|
"GET /github-apps/7/repositories": ok([
|
||||||
|
{ full_name: "heavy-duty/incubator" },
|
||||||
|
]),
|
||||||
|
});
|
||||||
|
const out = await registerGithubApp({
|
||||||
|
client: c.client,
|
||||||
|
name: "hdb-coolify-prod",
|
||||||
|
org: "heavy-duty",
|
||||||
|
orgRepo: "heavy-duty/incubator",
|
||||||
|
creds: creds(),
|
||||||
|
stateDir: tmp("cast-state-"),
|
||||||
|
log: () => {},
|
||||||
|
});
|
||||||
|
// The verify-only path: the list, then the check. No key upload, no App
|
||||||
|
// create — following the error message's own advice is now free.
|
||||||
|
expect(c.hits).toEqual([
|
||||||
|
"GET /github-apps",
|
||||||
|
"GET /github-apps/7/repositories",
|
||||||
|
]);
|
||||||
|
expect(out.coolifyAppId).toBe(7);
|
||||||
|
expect(out.keyUuid).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refuses when the name is taken by a DIFFERENT App rather than shadowing it", async () => {
|
||||||
|
const c = coolify({
|
||||||
|
"GET /github-apps": ok([
|
||||||
|
{ id: 7, name: "hdb-coolify-prod", app_id: 999999 },
|
||||||
|
]),
|
||||||
|
});
|
||||||
|
await expect(
|
||||||
|
registerGithubApp({
|
||||||
|
client: c.client,
|
||||||
|
name: "hdb-coolify-prod",
|
||||||
|
org: "heavy-duty",
|
||||||
|
orgRepo: "heavy-duty/incubator",
|
||||||
|
creds: creds(),
|
||||||
|
stateDir: tmp("cast-state-"),
|
||||||
|
log: () => {},
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/DIFFERENT App[\s\S]*github app id 999999/);
|
||||||
|
expect(c.hits).toEqual(["GET /github-apps"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refuses when duplicates ALREADY exist, because cast cannot pick one", async () => {
|
||||||
|
const c = coolify({
|
||||||
|
"GET /github-apps": ok([
|
||||||
|
{ id: 7, name: "dup", app_id: 12345 },
|
||||||
|
{ id: 8, name: "dup", app_id: 12345 },
|
||||||
|
]),
|
||||||
|
});
|
||||||
|
await expect(
|
||||||
|
registerGithubApp({
|
||||||
|
client: c.client,
|
||||||
|
name: "dup",
|
||||||
|
org: "o",
|
||||||
|
orgRepo: "o/r",
|
||||||
|
creds: creds(),
|
||||||
|
stateDir: tmp("cast-state-"),
|
||||||
|
log: () => {},
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/2 GitHub App records named dup[\s\S]*coolify ids: 7, 8/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("warns and proceeds when the list is unreadable — a bootstrap must not be blocked by a check", async () => {
|
||||||
|
const lines: string[] = [];
|
||||||
|
const c = coolify({
|
||||||
|
// No "GET /github-apps" route: the list 404s.
|
||||||
|
"POST /security/keys": ok({ uuid: "k" }),
|
||||||
|
"POST /github-apps": ok({ id: 3 }),
|
||||||
|
"GET /github-apps/3/repositories": ok([{ full_name: "o/r" }]),
|
||||||
|
});
|
||||||
|
const out = await registerGithubApp({
|
||||||
|
client: c.client,
|
||||||
|
name: "n",
|
||||||
|
org: "o",
|
||||||
|
orgRepo: "o/r",
|
||||||
|
creds: creds(),
|
||||||
|
stateDir: tmp("cast-state-"),
|
||||||
|
log: (l) => lines.push(l),
|
||||||
|
});
|
||||||
|
expect(out.coolifyAppId).toBe(3);
|
||||||
|
// Unreadable is not "empty" — it is said out loud, not assumed away.
|
||||||
|
expect(lines.join("\n")).toContain("could not list existing Coolify");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reads names off the list and ignores everything else", async () => {
|
||||||
|
const c = coolify({
|
||||||
|
"GET /github-apps": ok([
|
||||||
|
{ id: 1, name: "a" },
|
||||||
|
"not an object",
|
||||||
|
{ name: "wanted-but-no-id" },
|
||||||
|
{ id: 2, name: "wanted", app_id: 5 },
|
||||||
|
]),
|
||||||
|
});
|
||||||
|
expect(await findRegisteredApp(c.client, "wanted")).toEqual([
|
||||||
|
{ id: 2, appId: 5 },
|
||||||
|
]);
|
||||||
|
expect(await findRegisteredApp(c.client, "absent")).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// grok #3. Not a security boundary — the name is the operator's own — but a
|
||||||
|
// slash in it silently nests the credentials somewhere nobody will look, and
|
||||||
|
// `..` walks clean out of the state directory.
|
||||||
|
describe("the App name has to be usable as a filename", () => {
|
||||||
|
it("rejects separators, dot-references and empties before they become paths", () => {
|
||||||
|
for (const bad of ["", "a/b", "a\\b", "..", ".", "../escape", ".hidden"]) {
|
||||||
|
expect(() =>
|
||||||
|
persistCredentials({
|
||||||
|
stateDir: tmp("cast-state-"),
|
||||||
|
name: bad,
|
||||||
|
creds: creds(),
|
||||||
|
org: "o",
|
||||||
|
orgRepo: "o/r",
|
||||||
|
}),
|
||||||
|
).toThrow(/github app name/);
|
||||||
|
}
|
||||||
|
expect(() =>
|
||||||
|
preflightCredentialSlot({ stateDir: tmp("cast-state-"), name: "a/b" }),
|
||||||
|
).toThrow(/contains a path separator/);
|
||||||
|
// Ordinary names stay ordinary.
|
||||||
|
expect(() =>
|
||||||
|
persistCredentials({
|
||||||
|
stateDir: tmp("cast-state-"),
|
||||||
|
name: "hdb-coolify-prod",
|
||||||
|
creds: creds(),
|
||||||
|
org: "o",
|
||||||
|
orgRepo: "o/r",
|
||||||
|
}),
|
||||||
|
).not.toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("catches it at name resolution too, so a bad --name never reaches a network", () => {
|
||||||
|
const empty = loadBindings("", {
|
||||||
|
overrideText: [
|
||||||
|
"environments:",
|
||||||
|
" prod:",
|
||||||
|
" server: box",
|
||||||
|
" team: { id: 0, name: Root Team }",
|
||||||
|
"github_apps: {}",
|
||||||
|
"",
|
||||||
|
].join("\n"),
|
||||||
|
});
|
||||||
|
expect(() =>
|
||||||
|
resolveAppName({
|
||||||
|
bindings: empty,
|
||||||
|
orgRepo: "heavy-duty/incubator",
|
||||||
|
nameFlag: "../oops",
|
||||||
|
}),
|
||||||
|
).toThrow(/github app name/);
|
||||||
|
// And a hand-edited environments.yaml entry gets the same treatment: it
|
||||||
|
// becomes a filename by exactly the same route.
|
||||||
|
const bad = loadBindings("", {
|
||||||
|
overrideText: [
|
||||||
|
"environments:",
|
||||||
|
" prod:",
|
||||||
|
" server: box",
|
||||||
|
" team: { id: 0, name: Root Team }",
|
||||||
|
'github_apps: { "heavy-duty/incubator": "../oops" }',
|
||||||
|
"",
|
||||||
|
].join("\n"),
|
||||||
|
});
|
||||||
|
expect(() =>
|
||||||
|
resolveAppName({ bindings: bad, orgRepo: "heavy-duty/incubator" }),
|
||||||
|
).toThrow(/github app name/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// grok #4.
|
||||||
|
describe("cast identifies itself to GitHub", () => {
|
||||||
|
it("sends a User-Agent, because GitHub asks for one and 403s look like nothing else", async () => {
|
||||||
|
expect(githubUserAgent()).toMatch(/^cast\//);
|
||||||
|
const seen: Record<string, string>[] = [];
|
||||||
|
const fetchImpl = vi.fn(async (_url: string | URL, init?: RequestInit) => {
|
||||||
|
seen.push((init?.headers ?? {}) as Record<string, string>);
|
||||||
|
return new Response(JSON.stringify({ type: "Organization" }), {
|
||||||
|
status: 200,
|
||||||
|
});
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
await detectOwnerType("heavy-duty", fetchImpl);
|
||||||
|
await findInstallationId({
|
||||||
|
owner: "heavy-duty",
|
||||||
|
ownerType: "Organization",
|
||||||
|
jwt: "jwt",
|
||||||
|
fetchImpl,
|
||||||
|
}).catch(() => {});
|
||||||
|
await convertManifestCode("code", fetchImpl).catch(() => {});
|
||||||
|
expect(seen.length).toBe(3);
|
||||||
|
for (const headers of seen) {
|
||||||
|
expect(headers["User-Agent"]).toBe(githubUserAgent());
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("the optional org-admin preflight", () => {
|
describe("the optional org-admin preflight", () => {
|
||||||
it("passes on admin and refuses on anything else", () => {
|
it("passes on admin and refuses on anything else", () => {
|
||||||
expect(preflightOrgAdmin("heavy-duty", () => '{"role":"admin"}')).toEqual({
|
expect(preflightOrgAdmin("heavy-duty", () => '{"role":"admin"}')).toEqual({
|
||||||
|
|
@ -845,6 +1059,7 @@ describe("`create` falls through into `register` — one implementation, not two
|
||||||
}) as unknown as typeof fetch;
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
const c = coolify({
|
const c = coolify({
|
||||||
|
"GET /github-apps": () => [200, []],
|
||||||
"POST /security/keys": () => [200, { uuid: "key-uuid-1" }],
|
"POST /security/keys": () => [200, { uuid: "key-uuid-1" }],
|
||||||
"POST /github-apps": () => [200, { id: 11 }],
|
"POST /github-apps": () => [200, { id: 11 }],
|
||||||
"GET /github-apps/11/repositories": () => [
|
"GET /github-apps/11/repositories": () => [
|
||||||
|
|
@ -887,9 +1102,10 @@ describe("`create` falls through into `register` — one implementation, not two
|
||||||
});
|
});
|
||||||
|
|
||||||
const out = await flow;
|
const out = await flow;
|
||||||
// The fall-through, asserted as an identity of behaviour: the same three
|
// The fall-through, asserted as an identity of behaviour: the same four
|
||||||
// calls, in the same order, that the `register`-only test above pins.
|
// calls, in the same order, that the `register`-only test above pins.
|
||||||
expect(c.hits).toEqual([
|
expect(c.hits).toEqual([
|
||||||
|
"GET /github-apps",
|
||||||
"POST /security/keys",
|
"POST /security/keys",
|
||||||
"POST /github-apps",
|
"POST /github-apps",
|
||||||
"GET /github-apps/11/repositories",
|
"GET /github-apps/11/repositories",
|
||||||
|
|
@ -908,6 +1124,181 @@ describe("`create` falls through into `register` — one implementation, not two
|
||||||
).toBe(privateKeyPem);
|
).toBe(privateKeyPem);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The blocker all three reviewers raised on #124, pinned. Conversion
|
||||||
|
// SUCCEEDS — GitHub has minted the App and shown the private key for the only
|
||||||
|
// time it ever will — and then the install poll fails for every attempt. The
|
||||||
|
// old order held that payload in memory across the whole poll and wrote it
|
||||||
|
// only inside registerGithubApp, so this scenario destroyed it.
|
||||||
|
it("keeps the one-shot PEM and client secret when the install NEVER lands", async () => {
|
||||||
|
const conversion = {
|
||||||
|
id: 424242,
|
||||||
|
slug: "hdb-coolify-prod",
|
||||||
|
client_id: "Iv23liXYZ",
|
||||||
|
client_secret: "github-issued-secret",
|
||||||
|
webhook_secret: "github-issued-webhook",
|
||||||
|
pem: privateKeyPem,
|
||||||
|
owner: { login: "heavy-duty", type: "Organization" },
|
||||||
|
};
|
||||||
|
const githubFetch = vi.fn(async (url: string | URL) => {
|
||||||
|
const u = String(url);
|
||||||
|
if (u.endsWith("/conversions"))
|
||||||
|
return new Response(JSON.stringify(conversion), { status: 200 });
|
||||||
|
// Never installed. 404 on every single attempt, which is the state the
|
||||||
|
// poll is designed to wait out and eventually give up on.
|
||||||
|
if (u.includes("/installation"))
|
||||||
|
return new Response("{}", { status: 404 });
|
||||||
|
return new Response(JSON.stringify({ type: "Organization" }), {
|
||||||
|
status: 200,
|
||||||
|
});
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
const c = coolify({});
|
||||||
|
const state = tmp("cast-state-");
|
||||||
|
const err = await createGithubApp({
|
||||||
|
client: c.client,
|
||||||
|
orgRepo: "heavy-duty/incubator",
|
||||||
|
name: "hdb-coolify-prod",
|
||||||
|
stateDir: state,
|
||||||
|
port: 0,
|
||||||
|
deps: {
|
||||||
|
fetchImpl: githubFetch,
|
||||||
|
openUrl: (url) => {
|
||||||
|
if (url.startsWith("http://127.0.0.1")) {
|
||||||
|
const u = new URL(url);
|
||||||
|
fetch(url)
|
||||||
|
.then((r) => r.text())
|
||||||
|
.then((page) => {
|
||||||
|
const s = /state=([^"&]+)/.exec(page)?.[1] ?? "";
|
||||||
|
return fetch(`${u.origin}/callback?code=the-code&state=${s}`);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
},
|
||||||
|
sleep: async () => {},
|
||||||
|
runGh: () => '{"role":"admin"}',
|
||||||
|
log: () => {},
|
||||||
|
installAttempts: 3,
|
||||||
|
installIntervalMs: 1,
|
||||||
|
},
|
||||||
|
}).then(
|
||||||
|
() => undefined,
|
||||||
|
(e: Error) => e,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(err).toBeDefined();
|
||||||
|
// 1. The secrets GitHub shows exactly once are ON DISK.
|
||||||
|
const pem = join(state, "github-apps", "hdb-coolify-prod.pem");
|
||||||
|
const json = join(state, "github-apps", "hdb-coolify-prod.json");
|
||||||
|
expect(readFileSync(pem, "utf8")).toBe(privateKeyPem);
|
||||||
|
const saved = JSON.parse(readFileSync(json, "utf8"));
|
||||||
|
expect(saved.client_secret).toBe("github-issued-secret");
|
||||||
|
expect(saved.webhook_secret).toBe("github-issued-webhook");
|
||||||
|
expect(saved.app_id).toBe(424242);
|
||||||
|
// The one field that is legitimately unknown, and the only one GitHub will
|
||||||
|
// answer again as many times as it is asked.
|
||||||
|
expect(saved.installation_id).toBeNull();
|
||||||
|
|
||||||
|
// 2. The remedy MATCHES REALITY — it names the files that exist and the
|
||||||
|
// command that finishes the job, and it does not claim credentials are
|
||||||
|
// saved somewhere they are not.
|
||||||
|
const message = (err as Error).message;
|
||||||
|
expect(message).toContain("Nothing is lost");
|
||||||
|
expect(message).toContain(pem);
|
||||||
|
expect(message).toContain(json);
|
||||||
|
expect(message).toContain("cast github-app register");
|
||||||
|
expect(message).toContain("--app-id 424242");
|
||||||
|
expect(message).toContain("Do NOT re-run `create`");
|
||||||
|
|
||||||
|
// 3. Nothing was registered with Coolify, so there is no half-record to
|
||||||
|
// reconcile — only an App on GitHub awaiting its install.
|
||||||
|
expect(c.hits).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("backfills the installation id onto the pending record rather than refusing itself", async () => {
|
||||||
|
const state = tmp("cast-state-");
|
||||||
|
const pending: PendingAppCredentials = {
|
||||||
|
appId: 12345,
|
||||||
|
clientId: "Iv23liABCDEF",
|
||||||
|
clientSecret: "cs-secret",
|
||||||
|
webhookSecret: "wh-secret",
|
||||||
|
privateKeyPem: privateKeyPem ?? "PEM",
|
||||||
|
};
|
||||||
|
const args = { stateDir: state, name: "app", org: "o", orgRepo: "o/r" };
|
||||||
|
const { secretsPath } = persistCredentials({ ...args, creds: pending });
|
||||||
|
expect(JSON.parse(readFileSync(secretsPath, "utf8")).installation_id).toBe(
|
||||||
|
null,
|
||||||
|
);
|
||||||
|
|
||||||
|
// The completion `create` performs once the install lands. This is the ONE
|
||||||
|
// transition allowed without --force, because nothing irreplaceable moves.
|
||||||
|
persistCredentials({
|
||||||
|
...args,
|
||||||
|
creds: { ...pending, installationId: 5150 },
|
||||||
|
});
|
||||||
|
expect(JSON.parse(readFileSync(secretsPath, "utf8")).installation_id).toBe(
|
||||||
|
5150,
|
||||||
|
);
|
||||||
|
|
||||||
|
// And it really is only that one field: a different client secret arriving
|
||||||
|
// alongside a filled-in installation id is still a refusal.
|
||||||
|
expect(() =>
|
||||||
|
persistCredentials({
|
||||||
|
...args,
|
||||||
|
creds: { ...pending, installationId: 5150, clientSecret: "other" },
|
||||||
|
}),
|
||||||
|
).toThrow(/refusing to overwrite/);
|
||||||
|
// Nor does a KNOWN installation id get quietly replaced by a different one.
|
||||||
|
expect(() =>
|
||||||
|
persistCredentials({
|
||||||
|
...args,
|
||||||
|
creds: { ...pending, installationId: 6000 },
|
||||||
|
}),
|
||||||
|
).toThrow(/refusing to overwrite/);
|
||||||
|
});
|
||||||
|
|
||||||
|
// claude-bot's addition: the post-conversion persist must never be the thing
|
||||||
|
// that throws, because at that moment it is holding the only copy of the key.
|
||||||
|
it("refuses a name collision BEFORE the browser flow, when nothing can be lost", async () => {
|
||||||
|
const state = tmp("cast-state-");
|
||||||
|
persistCredentials({
|
||||||
|
stateDir: state,
|
||||||
|
name: "hdb-coolify-prod",
|
||||||
|
creds: creds({ privateKeyPem: "an older App's key" }),
|
||||||
|
org: "heavy-duty",
|
||||||
|
orgRepo: "heavy-duty/incubator",
|
||||||
|
});
|
||||||
|
|
||||||
|
const c = coolify({});
|
||||||
|
const githubFetch = vi.fn(async () => {
|
||||||
|
throw new Error("GitHub must not be reached");
|
||||||
|
}) as unknown as typeof fetch;
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
createGithubApp({
|
||||||
|
client: c.client,
|
||||||
|
orgRepo: "heavy-duty/incubator",
|
||||||
|
name: "hdb-coolify-prod",
|
||||||
|
stateDir: state,
|
||||||
|
port: 0,
|
||||||
|
ownerType: "Organization",
|
||||||
|
deps: {
|
||||||
|
fetchImpl: githubFetch,
|
||||||
|
runGh: () => '{"role":"admin"}',
|
||||||
|
log: () => {},
|
||||||
|
openUrl: () => false,
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/already has credentials on disk[\s\S]*register/);
|
||||||
|
|
||||||
|
// No browser flow, no App minted, no Coolify call — the whole point of
|
||||||
|
// checking now instead of after the conversion.
|
||||||
|
expect(c.hits).toEqual([]);
|
||||||
|
// And the older key is untouched.
|
||||||
|
expect(
|
||||||
|
readFileSync(join(state, "github-apps", "hdb-coolify-prod.pem"), "utf8"),
|
||||||
|
).toBe("an older App's key");
|
||||||
|
});
|
||||||
|
|
||||||
it("refuses before the browser dance when gh says you are not an org admin", async () => {
|
it("refuses before the browser dance when gh says you are not an org admin", async () => {
|
||||||
const c = coolify({});
|
const c = coolify({});
|
||||||
await expect(
|
await expect(
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue