diff --git a/api/domains.test.ts b/api/domains.test.ts new file mode 100644 index 0000000..57aa75c --- /dev/null +++ b/api/domains.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, test } from 'bun:test'; +import { issueWithDnsChallenge } from './domains'; + +/** + * The refusals, which are the part worth testing. + * + * The happy path writes to Postgres and talks to NPM, so a test of it would be a test of this machine. + * What can be silently WRONG is which requests get through: a certificate built for a domain Officer does + * not know, or one spanning two providers, fails deep inside NPM after a challenge record has already been + * placed — and the error that comes back is about certbot rather than about the mistake. + */ + +describe('issueWithDnsChallenge refuses before it can leave a mess', () => { + test('no names at all', async () => { + await expect(issueWithDnsChallenge({ domainNames: [] })).rejects.toThrow(/at least one domain/i); + }); + + test('a name that belongs to no stored domain', async () => { + await expect(issueWithDnsChallenge({ domainNames: ['not-a-domain-officer-knows.example'] })).rejects.toThrow( + /add the domain first/i, + ); + }); +}); diff --git a/api/domains.ts b/api/domains.ts index 2d7eacf..5d6e6e0 100644 --- a/api/domains.ts +++ b/api/domains.ts @@ -190,8 +190,8 @@ export async function removeCredential(provider: string): Promise { /** * Decrypted, for the code that places a challenge record. **Never serialise the result of this.** * - * Nothing calls it yet — issuing a certificate through DNS-01 is the next piece, and it is the only thing - * that should. It exists now so the storage above is provably usable rather than write-only. + * `issueWithDnsChallenge` below is the only caller and the only thing that should be — it is the whole + * point of storing these at all. */ export async function readCredentialFields(provider: string): Promise | null> { const [row] = await db @@ -201,3 +201,104 @@ export async function readCredentialFields(provider: string): Promise; } + +// ── Issuing a certificate with the stored DNS credential ──────────────────────────────────────────── + +/** + * Which stored domain owns a hostname. + * + * `private.officer.dev` belongs to the `officer.dev` row. Longest match wins, so a machine holding both + * `officer.dev` and `dev.officer.dev` attributes `a.dev.officer.dev` to the more specific one — which is + * the one whose provider is likelier to be right if they ever differ. + */ +export async function ownerOf(hostname: string): Promise { + const name = normaliseDomain(hostname); + const rows = await listDomains(); + return ( + rows + .filter((row) => name === row.domain || name.endsWith(`.${row.domain}`)) + .sort((a, b) => b.domain.length - a.domain.length)[0] ?? null + ); +} + +/** + * certbot wants an ini file; we hold a JSON object. This is the whole translation. + * + * Field names came from NPM's own `dns-providers` template when the key was entered, so they are already + * the ones certbot expects — nothing is renamed here, and nothing should be. + */ +const toIni = (fields: Record): string => + Object.entries(fields) + .map(([key, value]) => `${key} = ${value}`) + .join('\n'); + +export type IssueParams = { + domainNames: string[]; + /** + * How long to wait for the challenge record to be visible before asking Let's Encrypt to look. + * + * Two minutes by default, which is generous for Cloudflare and barely enough for GoDaddy — their TTLs + * are minutes, not seconds. Too short is the failure that wastes a Let's Encrypt attempt and reads as a + * credential problem, so the default errs long. + */ + propagationSeconds?: number; +}; + +/** + * Issue a certificate over DNS-01, using the credential Officer holds. + * + * **This is the one route in this plugin that is deliberately not a pass-through**, and the reason is the + * credential: it is never returned across the API, so no caller — agent or human — can assemble this + * request themselves. The platform decrypts it, puts it in the request, and hands back what NPM said. + * + * That is the same arrangement as the NPM token in `create-proxy.ts`, one level up: the thing that wants + * the work done is not the thing holding the password. + * + * ── Why this exists at all ── + * + * A hostname that only resolves on a VPN cannot be visited by Let's Encrypt, so HTTP-01 is impossible and + * a DNS record is the only proof it will accept. Every publicly-pointing name should use the plain + * `POST /npm/nginx/certificates` instead — it needs no credential and no waiting. + */ +export async function issueWithDnsChallenge(params: IssueParams): Promise<{ provider: string; body: unknown }> { + const names = params.domainNames.map(normaliseDomain).filter(Boolean); + if (names.length === 0) throw new DomainError('At least one domain name is required.', 400); + + // Every name must belong to the same provider: NPM takes ONE credential per certificate, so a + // certificate spanning two providers is not a thing that can be requested, and discovering that + // halfway through would leave a challenge record behind. + const owners = await Promise.all(names.map((name) => ownerOf(name))); + const missing = names.filter((_, index) => !owners[index]); + if (missing.length > 0) { + throw new DomainError( + `Not in the domain list: ${missing.join(', ')}. Add the domain first so Officer knows whose DNS it is.`, + 400, + ); + } + + const providers = [...new Set(owners.map((owner) => owner!.dnsProvider).filter(Boolean))]; + if (providers.length === 0) throw new DomainError('That domain has no recognised DNS provider.', 400); + if (providers.length > 1) { + throw new DomainError(`One certificate cannot span two DNS providers (${providers.join(', ')}).`, 400); + } + + const provider = providers[0]!; + const fields = await readCredentialFields(provider); + if (!fields) { + throw new DomainError(`No API key stored for ${provider}. Add one from the Domains section.`, 409); + } + + return { + provider, + body: { + provider: 'letsencrypt', + domain_names: names, + meta: { + dns_challenge: true, + dns_provider: provider, + dns_provider_credentials: toIni(fields), + propagation_seconds: params.propagationSeconds ?? 120, + }, + }, + }; +} diff --git a/api/router.ts b/api/router.ts index 6de53c9..286d39d 100644 --- a/api/router.ts +++ b/api/router.ts @@ -11,6 +11,7 @@ import { removeCredential, removeDomain, saveCredential, + issueWithDnsChallenge, DomainError, } from './domains'; @@ -325,6 +326,64 @@ router.delete('/domains/:id', async (ctx) => { return ctx.json({ ok: true }); }); +/** + * `POST /domains/certificate` — a certificate for a name Let's Encrypt cannot visit. + * + * **The one route here that is deliberately not a pass-through.** Everything else hands NPM's API through; + * this one cannot, because the request needs a DNS credential that is never returned to any caller. So the + * platform decrypts it, builds the request, and returns NPM's own answer unchanged. + * + * Same arrangement as the NPM token itself, one level up: the thing that wants the work done is not the + * thing holding the password. An agent asks for a certificate; it never sees the key that got it. + * + * **NOT under `/_config`.** The credential is owner-only to READ and WRITE, and that stays true — but + * USING it on the owner's behalf is the feature, and gating this route to the owner would defeat it. A + * `read`-level account still cannot reach it: this is a POST, and `isRequestAllowedAtLevel` refuses every + * unsafe method below `write`. + * + * For a publicly-resolving name, do NOT use this. `POST /npm/nginx/certificates` needs no credential and + * no propagation wait — see the skill. + */ +router.post('/domains/certificate', async (ctx) => { + const body = (await ctx.req.json().catch(() => null)) as + | { domain_names?: string[]; propagation_seconds?: number } + | null; + + const credentials = await readCredentials(); + if (!credentials) { + return ctx.json({ error: 'No Nginx Proxy Manager connection is configured.', unconfigured: true }, 409); + } + + let request: Awaited>; + try { + request = await issueWithDnsChallenge({ + domainNames: body?.domain_names ?? [], + propagationSeconds: body?.propagation_seconds, + }); + } catch (error) { + return domainFailure(ctx, error); + } + + try { + // A long timeout on purpose: NPM waits for the challenge record to propagate before it asks Let's + // Encrypt to look, so this legitimately takes minutes. The default 15s would abandon a request that + // is going to succeed — and abandoning it does not stop it, which is the worst of both. + const result = await npmProxy(credentials, { + method: 'POST', + path: '/nginx/certificates', + body: JSON.stringify(request.body), + timeoutMs: 10 * 60 * 1000, + }); + return new Response(result.body, { + status: result.status, + headers: { 'content-type': result.contentType ?? 'application/json' }, + }); + } catch (error) { + const shaped = asError(error); + return ctx.json(shaped.body, shaped.status); + } +}); + // ── DNS credentials — owner only, by the manifest's `/_config` prefix ──────────────────────────────── router.get('/_config/dns-providers', async (ctx) => ctx.json(await listCredentials())); diff --git a/skills/proxy-manager/SKILL.md b/skills/proxy-manager/SKILL.md index 454c017..138c398 100644 --- a/skills/proxy-manager/SKILL.md +++ b/skills/proxy-manager/SKILL.md @@ -111,10 +111,34 @@ it, and the only proof it accepts is a DNS record: } ``` -**You cannot fill in `dns_provider_credentials` yourself, and should not try.** Officer stores DNS -provider keys encrypted and never returns them — not to you, not to the owner. Reaching for one is the -refusal-shaped mistake this skill warns about below. The route that injects it server-side is not built -yet; until it is, a private hostname's certificate is something the owner does in NPM's own UI. +**You cannot fill in `dns_provider_credentials` yourself, and must not try.** Officer stores DNS provider +keys encrypted and never returns them — not to you, not to the owner. Reaching for one is the +refusal-shaped mistake this skill warns about. + +**So there is a different route for this, and it is the only one that works:** + +```bash +curl -s -X POST -H "Authorization: Bearer $OFFICER_AGENT_KEY" -H "Host: $OFFICER_API_HOST" \ + -H 'content-type: application/json' \ + -d '{"domain_names":["private.example.com"]}' \ + "$OFFICER_API_URL/api/proxy-manager/domains/certificate" +``` + +Note the path: `/domains/certificate`, **not** `/npm/…`. It is not a pass-through. The platform works out +which DNS provider owns that name from the Domains list, decrypts the stored key, builds the request above +and hands back NPM's own answer. You ask for a certificate and never see the key that got it. + +`propagation_seconds` may be passed and defaults to 120 — long, because a challenge record that has not +spread yet fails in a way that reads as a bad credential. **Expect this to take minutes**, not seconds, and +do not retry a request that has not come back. + +What it refuses, and what each refusal means: + +- **400 "Not in the domain list"** — the apex is not in Domains. Officer does not know whose DNS it is. + The owner adds it in the Domains section; you cannot. +- **409 "No API key stored"** — the domain is known, its provider is known, and no key has been entered. + Again the owner's to do, from the same section. +- **400 "cannot span two DNS providers"** — one certificate carries one credential. Split the request. ## Certificates, read the way Let's Encrypt behaves