@claude-bot-andresmgsl's outstanding item from the prior round, which my last
reply passed over in silence. That silence read as an oversight because it was
one.
`--port` on the create path was still bare `Number()`, so `--port abc` became
NaN, reached `server.listen(NaN)` in github-app.ts, and died as an uncaught
ERR_SOCKET_BAD_PORT stack trace — after `detectOwnerType` and the org-admin
preflight had already gone out. It is the same missing argv validation this
round fixed for the two ids, in a command whose stated rule is reject before
any write or network call.
Nothing is destroyed when it fails: no App and no client secret exist at that
point. So this is not about damage, it is about the command honouring its own
rule, and about failing with a sentence rather than a stack trace.
Range-checked as well as digits-only: `--port 99999` passes every test the ids
need and still cannot be listened on.
Scope, stated rather than assumed: `server add --port` (src/cli.ts:2430) has
the identical shape but predates this branch and is not in its diff. It is a
real instance of the same bug and belongs in its own change, not smuggled into
this one.
Four CLI cases — non-numeric, out-of-range, zero, decimal — asserting exit 2,
no stub hits and an unchanged state dir, driven through `create` because that
is the path that reads the flag. Verified by mutation: disabling the check
fails all four.
Both reviewers' blocker. `githubAppCommand` checked the two ids for truthiness
only, then handed them to `Number()`. `--app-id nope` becomes NaN, and
`JSON.stringify(NaN)` is `null` — so on a path that deliberately persists
BEFORE calling Coolify, a typo wrote a credential record with a null app_id
and could upload the security key before `POST /github-apps` rejected it. A
half-run leaving a corrupt record on disk and a stray key on the server.
Validated with the other ARGV checks, ABOVE openCoolify/assertTeam rather than
in the register branch where I first put it. The first placement still let
`GET /teams/current` go out before the refusal — the new test caught that,
which is the argument for asserting "no stub hits" rather than "no writes". A
typo should cost nothing, not one request.
Digits-only rather than Number.isInteger: `1e3` and `0x10` are integers to
JavaScript but are not how a GitHub App id is written, and quietly storing 1000
for `1e3` is the same class of wrong answer as storing null for `nope`.
Coverage asserts both halves the review asked for — no stub hit AND an
unchanged state directory — across non-numeric (both flags), zero, decimal,
exponent and hex.
A negative id gets its own case rather than joining the loop: parseArgs reads
the leading dash as an option and rejects `-5` as unknown, exiting 1 rather
than 2. The property that matters still holds — refused before any write or
request — but it is a different path with a different exit code, and a
loosened shared assertion would have hidden that rather than recorded it.
Verified by mutation: disabling the check fails all six loop cases.
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>
The GitHub App was the one piece of a Coolify instance cast could not
reproduce. There is no REST endpoint that creates one — no POST /apps, no
GraphQL mutation, no `gh app` subcommand, no PAT scope — so `create` runs the
only programmatic path there is: GitHub's App Manifest flow, a one-shot page
served on 127.0.0.1 whose form POST the operator's own browser session
authenticates, followed by an unauthenticated code exchange.
That exchange is the only moment GitHub yields the private key, the client
secret and the webhook secret together; all three are persisted to
<state>/github-apps/ at 0600 under a .gitignore of `*`.
`create` does not reimplement `register`: it obtains credentials and then calls
exactly that path. Both verbs end at GET /github-apps/{id}/repositories,
asserting the repo is actually reachable — the check that turns a silent
misconfiguration into an error next to the thing that caused it.
github_apps.<org>/<repo> in environments.yaml (--name only seeds an absent
entry, and is refused when it disagrees), the client secret is stdin-only, and
--webhook-secret is optional. scripts/register-github-app.sh is deleted.
No new dependencies: node:http for the callback, node:crypto's
createSign("RSA-SHA256") for the App JWT that recovers the installation id from
the App's own key rather than from a spoofable redirect parameter.
Closes#7
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
