diff --git a/README.md b/README.md index 5e08343..8fefa29 100644 --- a/README.md +++ b/README.md @@ -45,20 +45,88 @@ aem-mcp ``` ### Configuration + ``` Options: - --version Show version number [boolean] - -H, --host [string] [default: "http://localhost:4502"] - -u, --user [string] [default: "admin"] - -p, --pass [string] [default: "admin"] - -i, --id clientId [string] [default: ""] - -s, --secret clientSecret [string] [default: ""] - -m, --mcpPort [number] [default: 8502] - -h, --help Show help [boolean] + -H, --host [string] [default: "http://localhost:4502"] + -u, --user [string] [default: "admin"] + -p, --pass [string] [default: "admin"] + -i, --id clientId [string] [default: ""] + -s, --secret clientSecret [string] [default: ""] + -C, --cert path to client certificate PEM file for mTLS to AEM. [string] + -k, --key path to private key PEM file for mTLS to AEM. [string] + --ca path to CA bundle PEM file (self-signed AEM tenants). [string] + --cert-watch-interval-min poll cert mtime every N minutes; reload on change. 0 disables (default). + [number] [default: 0] + -m, --mcpPort [number] [default: 8502] + --bind host interface to bind. Default 127.0.0.1 (loopback only). + [string] [default: "127.0.0.1"] + --shutdown-drain-seconds max seconds to wait for in-flight requests on SIGINT/SIGTERM. + [number] [default: 60] + --allow-origin extra Origin header value to allow on /mcp (repeatable). + [array] [default: []] + -h, --help Show help [boolean] ``` -For AEMaaCS, use the `clientId` and `clientSecret` for authentication. [More info](https://developer.adobe.com/developer-console/docs/guides/authentication/ServerToServerAuthentication/implementation). -For self-hosted AEM use user/pass. The default credentials are `admin:admin`. +### Authentication modes + +The server supports three auth modes for talking to AEM. The factory picks +the strongest one available, with cert-auth taking priority over OAuth and +OAuth over Basic: + +| Mode | Flags | When to use | +|---|---|---| +| **Basic** | `-u/-p` (defaults `admin/admin`) | Local AEM, on-prem AEM where Basic is still enabled | +| **OAuth (Adobe IMS S2S)** | `-i -s ` | AEMaaCS tenants. [More info](https://developer.adobe.com/developer-console/docs/guides/authentication/ServerToServerAuthentication/implementation). | +| **mTLS (client certificate)** | `--cert --key [--ca ]` | Air-gapped AEM, enterprise mTLS gateways, compliance-driven tenants (PCI-DSS, FedRAMP, HIPAA), org-issued per-developer client certs | + +If you supply both `--cert/--key` and `--id/--secret`, cert-auth wins and a +warning is logged that OAuth params will be ignored. + +> **Trust boundary** — mTLS authenticates the **server → AEM** leg only. It +> does **not** authenticate the **MCP client → server** leg. The `/mcp` +> endpoint stays open to anyone who can reach the bind interface; the +> server defaults to loopback (`127.0.0.1`) precisely because of this. If +> you change `--bind` to a non-loopback address, you must put a reverse +> proxy with auth in front yourself. + +#### Encrypted private keys + +If your private key is PKCS#8-encrypted (`-----BEGIN ENCRYPTED PRIVATE KEY-----`), +supply the passphrase via the `AEM_KEY_PASSPHRASE` env var. There is +intentionally **no `--passphrase` CLI flag** — CLI arguments are visible +to any user on the machine through `ps aux`, env vars are not. For org +PKIs that mandate encrypted-key export (Venafi, internal CAs), this is +the supported path. + +#### Cert rotation (production deployments) + +Production PKIs (cert-manager, HashiCorp Vault, ACM) rotate client certs +every 60–90 days. Two rotation paths are supported, both without a +process restart: + +- **SIGHUP-driven** — replace the PEM files on disk, then + `kill -HUP `. The server re-reads the PEMs, validates the keypair, + and atomically swaps the cached `undici.Agent`. The previous Agent's + keep-alive sockets drain for 30 s before being destroyed. Bad PEM + material is rejected before the swap, so the old cert keeps serving. +- **mtime-driven** — start the server with `--cert-watch-interval-min N` + (env: `AEM_CERT_WATCH_INTERVAL_MIN`). Every N minutes the server polls + the cert file's mtime; on change, the same reload code path runs. + Off by default (`0`). + +Both paths emit a stderr line like +`[cert-reload] strategy reloaded: SHA256(old)= → SHA256(new)=` +so SREs can correlate rotations in their logs. + +#### Revocation (CRL / OCSP) + +The server does **not** perform CRL or OCSP revocation checks of the +AEM server cert. This is a deliberate scope decision — handle +revocation at the upstream layer (Dispatcher, reverse proxy, mTLS +gateway) where you already have central PKI configuration. The TLS +handshake still validates the AEM server cert chain against the CA +bundle (`--ca` or the OS trust store). #### Environment variables @@ -66,11 +134,39 @@ For self-hosted AEM use user/pass. The default credentials are `admin:admin`. |---|---| | `MCP_LOGGER` | Set to `true` to enable diagnostic logging on stdout (off by default — required off for MCP stdio clients). | | `MCP_USERNAME` / `MCP_PASSWORD` | Optional HTTP Basic auth gate on `POST /mcp` (only active when both are set). | +| `MCP_BIND` | Default bind interface (overrides built-in `127.0.0.1`). CLI `--bind` takes precedence. | +| `MCP_ALLOWED_ORIGINS` | Comma-separated extra `Origin` values allowed on `/mcp`. Inspector ports 6274/6277 on `localhost`/`127.0.0.1` are always allowed. | +| `MCP_SHUTDOWN_DRAIN_SECONDS` | Default SIGINT/SIGTERM drain budget. CLI `--shutdown-drain-seconds` takes precedence. | | `AEM_IMS_URL` | Override the Adobe IMS token endpoint. Defaults to `https://ims-na1.adobelogin.com/ims/token`. Set to `https://ims-eu1.adobelogin.com/ims/token` (EMEA) or `https://ims-jp1.adobelogin.com/ims/token` (APAC) for non-NA AEMaaCS tenants. | +| `AEM_CERT_PATH` / `AEM_KEY_PATH` / `AEM_CA_PATH` | Cert-auth paths (env-var alternatives to `--cert/--key/--ca`). | +| `AEM_KEY_PASSPHRASE` | Passphrase for an encrypted private key. **Env-only — no CLI equivalent.** | +| `AEM_CERT_WATCH_INTERVAL_MIN` | Default cert mtime poll interval in minutes. CLI `--cert-watch-interval-min` takes precedence. | + +> **Production recommendation** — prefer env vars over CLI flags for +> credential paths. Env vars are visible only to the process owner (and +> root) via `/proc//environ`; CLI args appear in `ps aux` for +> anyone on the host. + +### Example Commands -### Example Command ```sh +# Basic auth (default — local AEM) aem-mcp -u=user@domain.com -p=mypass -H=https://author-qa.domain.com + +# OAuth (AEMaaCS) +aem-mcp -i= -s= -H=https://author-pXXX.adobeaemcloud.com + +# mTLS with explicit CA bundle +aem-mcp --cert=/etc/aem-mcp/client.crt --key=/etc/aem-mcp/client.key \ + --ca=/etc/aem-mcp/ca.crt \ + -H=https://author.internal.example.com + +# mTLS via env vars (production — keeps paths out of ps aux) +export AEM_CERT_PATH=/etc/aem-mcp/client.crt +export AEM_KEY_PATH=/etc/aem-mcp/client.key +export AEM_CA_PATH=/etc/aem-mcp/ca.crt +export AEM_KEY_PASSPHRASE='...' # only if the key is encrypted +aem-mcp -H=https://author.internal.example.com --cert-watch-interval-min=15 ``` ### Add AEM MCP to AI IDE diff --git a/docs/phase1-smoke-audit.md b/docs/phase1-smoke-audit.md deleted file mode 100644 index a0ebfc6..0000000 --- a/docs/phase1-smoke-audit.md +++ /dev/null @@ -1,65 +0,0 @@ -# Phase 1 Smoke Test Audit — Transport Compliance & Bug Fixes - -**Date:** 2026-06-11 -**Branch:** fix/DDE-365-security-hardening -**Node:** v20.19.4 -**Build:** `npm run build` (esbuild, ESM) - ---- - -## Test Results - -| ID | Area | Command / Harness | Expected | Result | -|----|------|-------------------|----------|--------| -| T1 | Loopback binding (BF2) | `lsof -nP -iTCP:8520 -sTCP:LISTEN` | `127.0.0.1:8520 LISTEN` | **PASS** | -| T2 | Credential in `--host` rejected | `node dist/cli.js -H http://admin:admin@localhost:4502` | exit 1 + error message | **PASS** | -| T3 | Health check — live AEM | `GET /health` (AEM at localhost:4502) | `auth: authorized` | **PASS** | -| T4 | MCP initialize — session ID | `POST /mcp` initialize body | HTTP 200 + `Mcp-Session-Id` header | **PASS** | -| T5 | OAuth single-flight storm | `node src/test/smoke-single-flight.mjs` (4 assertions) | ALL PASS | **PASS** | -| T6 | Blocked origin (BF3) | `Origin: http://evil.test` | HTTP 403 | **PASS** | -| T6b | Port-fuzzing variant | `Origin: http://localhost:9999` | HTTP 403 | **PASS** | -| T7 | Inspector origins allowed | Origins `localhost/127.0.0.1` on ports 6274 + 6277 | HTTP 200 (all 4) | **PASS** | -| T8 | No `Origin` passthrough | `curl` without Origin header | HTTP 200 | **PASS** | -| T9 | Invalid JSON-RPC → 400 (BF4) | Missing `method` field; wrong `jsonrpc` version | HTTP 400 (both) | **PASS** | -| T10 | Stale session (BF5/leak #18) | `Mcp-Session-Id: 00000000-...` on non-initialize | HTTP 404 | **PASS** | -| T11 | SIGINT clean drain (BF8) | `test-leak-17.mjs` Test 1 | exit 0, `drain complete` in stderr | **PASS** | -| T12 | SIGTERM clean drain (BF8) | `test-leak-17.mjs` Test 2 | exit 0, `drain complete` in stderr | **PASS** | -| T13 | Drain deadline → exit 1 | `test-leak-17.mjs` Test 3 (`--shutdown-drain-seconds 1` + stuck request) | exit 1, `drain deadline reached` within ~1s | **PASS** | -| T14 | `uncaughtException` / `unhandledRejection` | `test-leak-17.mjs` Tests 7–8 | exit 1, `[fatal]` log | **PASS** | -| T15 | Raw password never in stderr | `grep SUPERSECRET_DO_NOT_LOG /tmp/t15-stderr.txt` | no match | **PASS** | - -**Total: 16/16 PASS** (T13 drain sub-tests + T14 fatal-error sub-tests covered by 19 assertions in `test-leak-17.mjs`) - ---- - -## Automated Test Harnesses - -| File | What it covers | Run | -|------|---------------|-----| -| `src/test/smoke-single-flight.mjs` | OAuth single-flight token mint (3 scenarios, 4 assertions) | `npm run build && node src/test/smoke-single-flight.mjs` | -| `src/test/test-leak-17.mjs` | SIGINT/SIGTERM drain, drain deadline, uncaughtException, unhandledRejection (8 scenarios, 19 assertions) | `npm run build && node src/test/test-leak-17.mjs` | -| `src/test/test-leak-18.mjs` | Stale session → 404, error code -32001, re-initialize path (4 scenarios, 8 assertions) | `npm run build && node src/test/test-leak-18.mjs` | - ---- - -## Findings - -### ✅ T10 — Stale session now returns 404 (fixed as leak #18) - -- **Location:** `src/mcp/mcp.server-handler.ts:66` -- **Was:** `res.status(400)` with JSON-RPC code `-32000` -- **Fixed:** `res.status(404)` with JSON-RPC code `-32001` and message "Session not found." -- **Impact:** MCP Inspector auto-reinitialization loop now triggers correctly on server restart. -- **Test:** `src/test/test-leak-18.mjs` — 8/8 assertions PASS - -### ℹ Accept header requirement (SDK enforcement, not a bug) - -The MCP SDK's `StreamableHTTPServerTransport` returns **406 Not Acceptable** when the `POST /mcp` request is missing `Accept: application/json, text/event-stream`. This is correct per the MCP StreamableHTTP spec. All runbook `curl` commands must include this header — commands without it are invalid clients, not a server bug. - -Correct curl baseline: -```sh -curl -X POST http://127.0.0.1:8502/mcp \ - -H 'Content-Type: application/json' \ - -H 'Accept: application/json, text/event-stream' \ - -d '{...}' -``` \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 79d28df..b289493 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,9 @@ "@modelcontextprotocol/sdk": "^1.17.3", "cors": "^2.8.5", "express": "^5.1.0", - "yargs": "^18.0.0" + "undici": "7.27.2", + "yargs": "^18.0.0", + "zod": "3.25.76" }, "bin": { "aem-mcp": "dist/cli.js" @@ -1959,6 +1961,15 @@ "node": ">=14.17" } }, + "node_modules/undici": { + "version": "7.27.2", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.27.2.tgz", + "integrity": "sha512-uZsKNuzQxDMUY6M3pIMvy5tvlGmtq8XJ2oLAkfRKGNu+1VQAIvLy2xIVG5ATZl5wDXl/tddByAWCizRbOme+TA==", + "license": "MIT", + "engines": { + "node": ">=20.18.1" + } + }, "node_modules/undici-types": { "version": "7.10.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.10.0.tgz", @@ -2060,9 +2071,9 @@ } }, "node_modules/zod": { - "version": "4.4.3", - "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", - "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", + "version": "3.25.76", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", + "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", "funding": { "url": "https://github.com/sponsors/colinhacks" @@ -3294,6 +3305,11 @@ "integrity": "sha512-CWBzXQrc/qOkhidw1OzBTQuYRbfyxDXJMVJ1XNwUHGROVmuaeiEm3OslpZ1RV96d7SKKjZKrSJu3+t/xlw3R9A==", "dev": true }, + "undici": { + "version": "7.27.2", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.27.2.tgz", + "integrity": "sha512-uZsKNuzQxDMUY6M3pIMvy5tvlGmtq8XJ2oLAkfRKGNu+1VQAIvLy2xIVG5ATZl5wDXl/tddByAWCizRbOme+TA==" + }, "undici-types": { "version": "7.10.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.10.0.tgz", @@ -3363,9 +3379,9 @@ "integrity": "sha512-rwu/ClNdSMpkSrUb+d6BRsSkLUq1fmfsY6TOpYzTwvwkg1/NRG85KBy3kq++A8LKQwX6lsu+aWad+2khvuXrqw==" }, "zod": { - "version": "4.4.3", - "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", - "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==" + "version": "3.25.76", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", + "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==" }, "zod-to-json-schema": { "version": "3.25.2", diff --git a/package.json b/package.json index 614d239..1405631 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,9 @@ "@modelcontextprotocol/sdk": "^1.17.3", "cors": "^2.8.5", "express": "^5.1.0", - "yargs": "^18.0.0" + "undici": "7.27.2", + "yargs": "^18.0.0", + "zod": "3.25.76" }, "devDependencies": { "@types/cors": "^2.8.19", diff --git a/src/aem/aem.auth.schemas.ts b/src/aem/aem.auth.schemas.ts new file mode 100644 index 0000000..20bd97f --- /dev/null +++ b/src/aem/aem.auth.schemas.ts @@ -0,0 +1,33 @@ +import { z } from 'zod'; + +// ---------------------------------------------------------------------------- +// CertParamsSchema — shape validation for cert-auth CLI / env inputs (feat #4) +// ---------------------------------------------------------------------------- +// +// Validates the cert-auth inputs BEFORE they reach `CertAuthStrategy.init()`. +// Shape-only — file reads, PEM-format checks, keypair validation, encrypted- +// key detection, and Agent construction all live in `CertAuthStrategy.init()` +// (feat #3). This schema catches the cheap-to-detect mistakes: +// - Empty-string flags (`--cert ""`) +// - Partial config (cert without key, or key without cert) +// +// `passphrase` intentionally has NO matching CLI flag — it MUST be sourced from +// the `AEM_KEY_PASSPHRASE` env var to keep the secret out of `ps aux` and +// shell history. The CLI surface only exposes `cert`, `key`, `ca`. +// +// Callers should use `safeParse(...)`, never `.parse()`, and must NOT echo +// `error.issues[].received` back to stdout/stderr — that would leak path +// values into CI logs. + +export const CertParamsSchema = z + .object({ + cert: z.string().min(1).optional(), + key: z.string().min(1).optional(), + ca: z.string().min(1).optional(), + passphrase: z.string().min(1).optional(), + }) + .refine((d) => (d.cert && d.key) || (!d.cert && !d.key), { + message: '--cert and --key must be provided together', + }); + +export type CertParams = z.infer; diff --git a/src/aem/aem.auth.ts b/src/aem/aem.auth.ts index 6043071..2c2945f 100644 --- a/src/aem/aem.auth.ts +++ b/src/aem/aem.auth.ts @@ -1,3 +1,9 @@ +import fs from 'node:fs'; +import tls from 'node:tls'; +import crypto from 'node:crypto'; +import { Agent, Dispatcher } from 'undici'; +import { LOGGER } from '../utils/logger.js'; + // IMS endpoint defaults to NA. Override with AEM_IMS_URL for EMEA // (https://ims-eu1.adobelogin.com/ims/token) or APAC (https://ims-jp1.adobelogin.com/ims/token). const IMS_URL = process.env.AEM_IMS_URL || "https://ims-na1.adobelogin.com/ims/token"; @@ -47,3 +53,560 @@ export async function getAccessToken(clientId: string, clientSecret: string, sco return res.json(); } + +// ---------------------------------------------------------------------------- +// Auth Strategy interface + implementations +// ---------------------------------------------------------------------------- +// +// AuthStrategy encapsulates how the server authenticates to AEM. Three concrete +// strategies exist (or will exist): +// - BasicAuthStrategy : username/password, sent as `Authorization: Basic ...` +// - OAuthStrategy : Adobe IMS S2S, sent as `Authorization: Bearer ...` +// - CertAuthStrategy : mTLS client cert handshake; no Authorization header +// (added in feat #3) +// +// In feat #1 the interface + Basic/OAuth strategies are introduced; AEMFetch +// delegates to them internally. feat #2 completes the refactor by removing the +// AEMAuth union and consuming AuthStrategy directly from AEMFetch. + +export type AuthFactoryInput = { + username?: string; + password?: string; + clientId?: string; + clientSecret?: string; + scope?: string | string[]; + certPath?: string; + keyPath?: string; + caPath?: string; + passphrase?: string; +}; + +export interface AuthStrategy { + /** + * Headers to merge into outgoing AEM requests (e.g., `Authorization`). + * Cert strategy returns `{}` since identity lives in the TLS handshake. + */ + getHeaders(): Promise>; + + /** + * Optional undici `Dispatcher` (Agent) — only set by `CertAuthStrategy` for + * mTLS. Header-based strategies return undefined; the caller falls back to + * the default global Dispatcher. Typed as `Dispatcher` from undici 7.x. + */ + getAgent?(): Dispatcher | undefined; + + /** + * One-time idempotent setup. Called once by `AEMFetch.init()`. For + * `OAuthStrategy` this primes the token cache via the first IMS mint. For + * `CertAuthStrategy` (feat #3) this reads PEM files from disk, validates + * them, and builds the singleton `undici.Agent`. `BasicAuthStrategy` does + * not implement this — the encoded credential is computed in its + * constructor. + */ + init?(): Promise; + + /** + * Force a credential refresh on 401. Only `OAuthStrategy` implements this + * (mint a fresh IMS token). `BasicAuthStrategy` deliberately does not — re- + * encoding the same credentials produces the same Base64, so a 401-retry is + * a wasted round-trip and the `request()` path short-circuits when + * `refresh` is undefined. `CertAuthStrategy` also does not implement this — + * mTLS cert rotation is SIGHUP-driven via `reload()` (feat #7), not 401- + * driven; re-reading the same PEM files on a TLS failure would burn disk + * I/O without changing the handshake material. + */ + refresh?(): Promise; + + /** + * Release resources on shutdown. Only `CertAuthStrategy` implements this + * (destroy the cached `undici.Agent` socket pool, releasing keep-alive + * sockets ahead of `process.exit`). Called by the graceful drain path in + * `app.server.ts` (feat #6). + */ + destroy?(): Promise; +} + +export class BasicAuthStrategy implements AuthStrategy { + /** + * Precomputed base64 of `user:pass` in Latin-1 (ISO-8859-1) — AEM Sling + * decodes Basic credentials as Latin-1, not UTF-8. Encoding the source as + * 'latin1' keeps ASCII identical while making 0x80-0xFF code points + * (é/ü/ñ/etc.) round-trip correctly. Passwords with code points > 0xFF still + * can't be expressed in Basic auth and are out of scope. + */ + readonly encodedToken: string; + + constructor(username: string, password: string) { + if (!username || !password) { + throw new Error('BasicAuthStrategy requires both username and password'); + } + this.encodedToken = Buffer.from(`${username}:${password}`, 'latin1').toString('base64'); + } + + async getHeaders(): Promise> { + return { Authorization: `Basic ${this.encodedToken}` }; + } +} + +export class OAuthStrategy implements AuthStrategy { + private token: string = ''; + private tokenExpiry: number = 0; + private inflightToken: Promise | null = null; + private readonly clientId: string; + private readonly clientSecret: string; + private readonly scope?: string | string[]; + + constructor(clientId: string, clientSecret: string, scope?: string | string[]) { + if (!clientId || !clientSecret) { + throw new Error('OAuthStrategy requires both clientId and clientSecret'); + } + this.clientId = clientId; + this.clientSecret = clientSecret; + this.scope = scope; + } + + async getHeaders(): Promise> { + const token = await this.ensureToken(); + return { Authorization: `Bearer ${token}` }; + } + + /** + * Prime the token cache at startup. Idempotent — repeated calls with a + * valid cached token return immediately. Called by `AEMFetch.init()`. + */ + async init(): Promise { + await this.ensureToken(); + } + + /** + * Force a fresh IMS mint, discarding any cached token. Called from + * `AEMFetch.refreshAuthToken()` after a 401 with `expired_token`. + */ + async refresh(): Promise { + this.token = ''; + this.tokenExpiry = 0; + await this.ensureToken(); + } + + /** + * Returns the current bearer token, minting a new one when the cache is + * empty or expired. Single-flight dedup: concurrent post-expiry callers + * share a single IMS round-trip via `inflightToken`. + */ + async ensureToken(): Promise { + const now = Date.now(); + if (this.token && now < this.tokenExpiry) { + return this.token; + } + // Dedup concurrent mints: if another caller has already kicked off the + // IMS request, ride on its promise instead of issuing a parallel mint. + // Without this, N concurrent post-expiry callers trigger N IMS calls. + if (this.inflightToken) { + return this.inflightToken; + } + this.inflightToken = (async () => { + try { + const token = await getAccessToken(this.clientId, this.clientSecret, this.scope); + // Reject expires_in <= 60: a value at-or-below the 60s headroom would place + // tokenExpiry in the past, forcing an IMS mint on every request. Single-flight + // dedups within a tick but still burns a round-trip per call. NaN/undefined + // fail this check too (NaN > 60 is false). + if (!(token.expires_in > 60)) { + throw new Error( + `IMS returned invalid expires_in (${token.expires_in}); must be > 60 seconds to leave refresh headroom.` + ); + } + this.token = token.access_token; + this.tokenExpiry = now + (token.expires_in - 60) * 1000; + return this.token; + } finally { + this.inflightToken = null; + } + })(); + return this.inflightToken; + } +} + +// ---------------------------------------------------------------------------- +// CertAuthStrategy — mTLS via client certificate handshake (feat #3) +// ---------------------------------------------------------------------------- +// +// The mTLS identity lives entirely in the TLS handshake; no `Authorization` +// header is sent. `getAgent()` returns a cached `undici.Agent` (singleton, +// built once in `init()`) whose `connect` options carry the cert/key/CA/ +// passphrase + `minVersion: 'TLSv1.2'`. AEMFetch passes that Agent to +// Node's native `fetch` via the `dispatcher` field. +// +// Wiring CLI flags + factory selection lands in feat #4 / feat #5; in feat #3 +// the class is exported and instantiated directly by tests. + +export type CertAuthParams = { + /** Path to the client certificate PEM file. Read once in `init()`. */ + certPath: string; + /** Path to the private key PEM file. Read once in `init()`. */ + keyPath: string; + /** Optional path to a CA bundle PEM file (for self-signed AEM tenants). */ + caPath?: string; + /** + * Optional passphrase for an encrypted private key. Per the cert-auth plan + * this is read ONLY from `AEM_KEY_PASSPHRASE` env var (never a CLI flag) + * to keep secrets out of `ps aux` — the wiring happens in feat #4. + */ + passphrase?: string; +}; + +const MAX_PEM_BYTES = 1_048_576; // 1 MB; rejects accidental binary blobs / DoS +const PEM_BEGIN_PREFIX = '-----BEGIN '; +const ENCRYPTED_KEY_MARKER = '-----BEGIN ENCRYPTED PRIVATE KEY-----'; + +// Module-level registry of live `CertAuthStrategy` instances (feat #6). The +// per-session architecture in `mcp.server-handler.ts` creates one AEMConnector +// per MCP session, plus one global connector in `app.server.ts` for /health — +// so a single process holds N strategies, not one. The registry lets the +// graceful-drain path (`app.server.ts`) destroy all of them on SIGINT/SIGTERM +// without coupling shutdown to session bookkeeping. Instances self-register +// at the end of `init()` (after the Agent is built) and self-unregister in +// `destroy()`. The set is intentionally private to this module — callers go +// through `destroyAllCertStrategies()` / `reloadAllCertStrategies()`. +const liveCertStrategies: Set = new Set(); + +// Old-Agent drain window during a cert reload (feat #7). The fresh Agent +// takes new requests immediately after the atomic swap, but in-flight +// requests started on the old Agent need a grace period to finish before +// we destroy() it. 30s mirrors the plan and matches typical AEM read +// latencies while staying well under the leak #17 shutdown drain (60s). +const RELOAD_OLD_AGENT_DRAIN_MS = 30_000; + +export class CertAuthStrategy implements AuthStrategy { + private cachedAgent: Agent | null = null; + // SHA-256 of the current cert PEM (hex). Set in `init()`/`reload()` so + // `reload()` can log the old → new transition without re-reading the file + // a second time just to fingerprint it. Public-readable for tests; the + // value is non-sensitive (a public-key hash) so leakage is harmless. + certFingerprint: string = ''; + // Drain timer for the previous Agent after reload(). Stored on the instance + // so rapid successive reloads can cancel the previous timer before setting a + // new one — prevents the first timer from firing on the now-live agent. + private drainTimer: ReturnType | undefined; + private readonly params: CertAuthParams; + + constructor(params: CertAuthParams) { + if (!params.certPath || !params.keyPath) { + throw new Error('CertAuthStrategy requires both certPath and keyPath'); + } + this.params = params; + } + + /** + * Returns an empty header set — mTLS identity is carried by the TLS + * handshake material, not an `Authorization` header. + */ + async getHeaders(): Promise> { + return {}; + } + + /** + * Returns the cached singleton `undici.Agent` built in `init()`. NEVER + * constructs a new Agent here — per-request Agent construction allocates a + * fresh keep-alive socket pool every call, leaking FDs until `ulimit -n` is + * exhausted (mcp-cert-auth-plan.md §"🔴 HIGH — undici.Agent mora biti + * keširani singleton"). + */ + getAgent(): Dispatcher | undefined { + return this.cachedAgent ?? undefined; + } + + /** + * One-time setup: read + validate PEM files, build the singleton Agent. + * Safe to call only once per instance — repeated calls throw to prevent + * silently orphaning the previous socket pool. The rotation path (feat #7) + * uses an explicit `reload()` with an atomic swap + 30s drain. + */ + async init(): Promise { + if (this.cachedAgent) throw new Error('CertAuthStrategy.init() already called — use reload() to rotate'); + const { certPath, keyPath, caPath, passphrase } = this.params; + + const cert = this.readAndGuardPem(certPath, 'cert'); + const key = this.readAndGuardPem(keyPath, 'key'); + const ca = caPath ? this.readAndGuardPem(caPath, 'CA') : undefined; + + // Encrypted-key guard: detect `-----BEGIN ENCRYPTED PRIVATE KEY-----` + // before tls.createSecureContext throws an unreadable OpenSSL trace. + if (key.includes(ENCRYPTED_KEY_MARKER) && !passphrase) { + throw new Error('Encrypted PEM key requires AEM_KEY_PASSPHRASE env variable'); + } + + // Keypair consistency: cert and key must belong to the same pair. + // `tls.createSecureContext` throws synchronously on mismatch — re-throw + // with a sanitized message (no OpenSSL trace, no file paths). + try { + tls.createSecureContext({ cert, key, passphrase }); + } catch { + throw new Error('Certificate and private key do not match (keypair mismatch)'); + } + + // World-readable key warning (defense-in-depth — not a hard reject so + // ephemeral CI/secret-mount scenarios continue to work). + try { + const mode = fs.statSync(keyPath).mode; + if ((mode & 0o004) !== 0) { + LOGGER.warn('Private key file is world-readable. Consider `chmod 600` for production deployments.'); + } + } catch { + // already-failing readAndGuardPem would have surfaced this earlier + } + + // Singleton Agent. `connect` is the tls.connect options bag; passing + // `minVersion: 'TLSv1.2'` explicitly because Node's default still allows + // TLS 1.0/1.1 in some build configurations — unacceptable for mTLS. + // NEVER set `rejectUnauthorized: false` (defeats the entire mTLS chain). + this.cachedAgent = new Agent({ + connect: { cert, key, ca, passphrase, minVersion: 'TLSv1.2' }, + }); + this.certFingerprint = sha256Hex(cert); + + // Register only after the Agent is built — a failed init() must not + // leave a half-initialized strategy in the registry. + liveCertStrategies.add(this); + } + + /** + * Re-read PEM material and atomically swap the cached Agent (feat #7). + * Used for rotation when the on-disk certs have been replaced (cert- + * manager, Vault) — operators trigger via SIGHUP, or the optional mtime + * poll detects the mtime change. + * + * Sequence: + * 1. Re-read PEMs through `readAndGuardPem` (same path-traversal / 1 MB + * / BEGIN-prefix guards as `init()`). + * 2. Detect encrypted-key marker; require passphrase. + * 3. `tls.createSecureContext` to validate the keypair BEFORE building a + * new Agent — if the new material is broken, throw and leave the old + * Agent untouched (no broken state). + * 4. Build the new Agent. + * 5. Atomic swap (single JS assignment is atomic — JS is single-threaded). + * 6. Update `certFingerprint`. + * 7. Log the old → new fingerprint transition. + * 8. After `RELOAD_OLD_AGENT_DRAIN_MS` (30s), destroy the old Agent so + * its keep-alive sockets close. Fire-and-forget — the new Agent is + * already serving new requests, and the destroy timer does not block + * `reload()` from returning. + * + * Returns the new fingerprint (caller can correlate logs). + */ + async reload(): Promise<{ oldFingerprint: string; newFingerprint: string }> { + if (!this.cachedAgent) { + throw new Error('CertAuthStrategy.reload() called before init()'); + } + + const { certPath, keyPath, caPath, passphrase } = this.params; + + // Step 1–3: read + validate. Throws on bad material; old Agent stays. + const cert = this.readAndGuardPem(certPath, 'cert'); + const key = this.readAndGuardPem(keyPath, 'key'); + const ca = caPath ? this.readAndGuardPem(caPath, 'CA') : undefined; + if (key.includes(ENCRYPTED_KEY_MARKER) && !passphrase) { + throw new Error('Encrypted PEM key requires AEM_KEY_PASSPHRASE env variable'); + } + try { + tls.createSecureContext({ cert, key, passphrase }); + } catch { + throw new Error('Certificate and private key do not match (keypair mismatch)'); + } + + // Step 4–6: build new, swap, fingerprint. Capture old refs FIRST so a + // concurrent reload that overlapping JS-tick-scheduled the same swap + // can't lose the previous Agent. + const oldAgent = this.cachedAgent; + const oldFingerprint = this.certFingerprint; + const newAgent = new Agent({ + connect: { cert, key, ca, passphrase, minVersion: 'TLSv1.2' }, + }); + const newFingerprint = sha256Hex(cert); + this.cachedAgent = newAgent; + this.certFingerprint = newFingerprint; + + // Step 7: log. Stderr (unconditional) — rotation must be visible even + // without MCP_LOGGER. SHA-256 of a cert is a public artifact; safe to log. + process.stderr.write( + `[cert-reload] strategy reloaded: SHA256(old)=${shortHash(oldFingerprint)} → SHA256(new)=${shortHash(newFingerprint)}\n` + ); + + // Step 8: schedule old-Agent destroy after drain window. Cancel any + // previous drain timer first — if reload() is called again within the + // 30s window, the first timer must not fire on the now-live agent. + // unref() so the timer alone doesn't keep the process alive on shutdown. + if (this.drainTimer !== undefined) { + clearTimeout(this.drainTimer); + } + this.drainTimer = setTimeout(() => { + this.drainTimer = undefined; + oldAgent.destroy().catch(() => { /* best effort */ }); + }, RELOAD_OLD_AGENT_DRAIN_MS); + this.drainTimer.unref(); + + return { oldFingerprint, newFingerprint }; + } + + /** + * Release the keep-alive socket pool. Called by the graceful drain path + * (feat #6) before `process.exit` so lingering connections don't confuse + * `lsof`-based leak detectors. Idempotent — repeated calls are no-ops and + * the unregister step uses `Set.delete` which is itself idempotent. + */ + async destroy(): Promise { + if (this.cachedAgent) { + await this.cachedAgent.destroy(); + this.cachedAgent = null; + } + liveCertStrategies.delete(this); + } + + /** + * Read a PEM file with all defensive guards. Returns the file contents as + * a Buffer (binary-safe). Errors are sanitized to omit filesystem paths so + * they don't leak through `handleAEMHttpError` to MCP clients. + * + * Guards (in order — cheapest first): + * 1. Path traversal: reject any `..` segment in the user-supplied path + * BEFORE `path.resolve` flattens it. `/etc/ssl/../shadow` would + * otherwise silently resolve to `/etc/shadow`. + * 2. File size: reject > 1 MB (PEM bundles are well under 100 KB; a + * multi-MB file is either an accident or a DoS attempt). + * 3. PEM format: first 64 bytes must start with `-----BEGIN ` — catches + * binary blobs, plain text, and `echo not-pem > x.pem` mistakes. + */ + private readAndGuardPem(p: string, kind: 'cert' | 'key' | 'CA'): Buffer { + const segments = p.split(/[/\\]/); + if (segments.includes('..')) { + throw new Error(`Path traversal detected in ${kind} path`); + } + + let size: number; + try { + size = fs.statSync(p).size; + } catch { + throw new Error(`${kind} file not found or not readable`); + } + + if (size > MAX_PEM_BYTES) { + throw new Error(`${kind} file too large (limit: 1 MB)`); + } + + let buf: Buffer; + try { + buf = fs.readFileSync(p); + } catch { + throw new Error(`${kind} file not readable`); + } + + const head = buf.subarray(0, 64).toString('utf8'); + if (!head.startsWith(PEM_BEGIN_PREFIX)) { + throw new Error(`Not a PEM-encoded ${kind} file`); + } + + return buf; + } +} + +/** + * SHA-256 of a buffer as a lowercase hex string. Used for cert fingerprint + * logging in `init()` and `reload()`. + */ +function sha256Hex(buf: Buffer): string { + return crypto.createHash('sha256').update(buf).digest('hex'); +} + +/** + * Truncate a hex fingerprint for log readability — full 64-hex digest is + * noisy; first 16 chars (64 bits) is still uniquely identifying for any + * realistic cert population. Operators who need the full value can grep + * the cert file with `openssl x509 -fingerprint -sha256 -noout -in cert.pem`. + */ +function shortHash(hex: string): string { + return hex.slice(0, 16); +} + +/** + * Destroy every live `CertAuthStrategy` registered in this process. Called + * from the graceful drain path in `app.server.ts` (feat #6) so cached + * `undici.Agent` socket pools release before `process.exit`. Returns the + * number of strategies destroyed so the drain logger can report it. + * + * Errors inside a single `destroy()` are swallowed (best-effort) — one + * stuck Agent must not prevent the others from cleaning up. The drain path + * additionally races this call against a small budget to bound total time. + */ +export async function destroyAllCertStrategies(): Promise { + const snapshot = Array.from(liveCertStrategies); + await Promise.all( + snapshot.map((s) => s.destroy().catch(() => { /* best-effort */ })) + ); + return snapshot.length; +} + +/** + * Reload every live `CertAuthStrategy` registered in this process (feat #7). + * Called from the SIGHUP handler and from the optional mtime poll in + * `app.server.ts`. Returns the count of successful reloads and a list of any + * errors so the caller can log per-strategy failures without aborting the + * whole rotation. + * + * If a single strategy's reload throws (bad PEM, keypair mismatch on the + * refreshed material, etc.), that strategy keeps its OLD Agent — no broken + * state. Other strategies still get reloaded. + */ +export async function reloadAllCertStrategies(): Promise<{ reloaded: number; errors: string[] }> { + const snapshot = Array.from(liveCertStrategies); + const errors: string[] = []; + let reloaded = 0; + await Promise.all( + snapshot.map(async (s) => { + try { + await s.reload(); + reloaded += 1; + } catch (e: any) { + errors.push(e?.message ?? String(e)); + } + }) + ); + return { reloaded, errors }; +} + +/** + * Resolve which auth strategy to use based on supplied credentials. + * - `certPath` + `keyPath` → `CertAuthStrategy` (highest priority) + * - `clientId` + `clientSecret` → `OAuthStrategy` + * - `username` + `password` → `BasicAuthStrategy` + * - otherwise → throws + * + * Conflict resolution: if cert + key are supplied alongside OAuth credentials + * (either `clientId` or `clientSecret`), cert-auth wins and a warning is + * logged so the operator knows the OAuth params were ignored. We deliberately + * do NOT warn for cert + Basic because Basic credentials default to + * `admin/admin` from yargs — we can't distinguish "explicit" from "default" + * without threading additional metadata through the API, and the noise would + * cost more than the signal. + */ +export function createAuthStrategy(input: AuthFactoryInput): AuthStrategy { + if (input.certPath && input.keyPath) { + if (input.clientId || input.clientSecret) { + process.stderr.write( + '[auth] WARNING: both cert and OAuth params supplied — cert takes precedence; OAuth params ignored\n' + ); + } + return new CertAuthStrategy({ + certPath: input.certPath, + keyPath: input.keyPath, + caPath: input.caPath, + passphrase: input.passphrase, + }); + } + if (input.clientId && input.clientSecret) { + return new OAuthStrategy(input.clientId, input.clientSecret, input.scope); + } + if (input.username && input.password) { + return new BasicAuthStrategy(input.username, input.password); + } + throw new Error('No authentication credentials provided'); +} diff --git a/src/aem/aem.connector.ts b/src/aem/aem.connector.ts index 8dd651b..0fe5734 100644 --- a/src/aem/aem.connector.ts +++ b/src/aem/aem.connector.ts @@ -1,7 +1,8 @@ import { AEMConfig, getAEMConfig, isValidContentPath, isValidLocale } from './aem.config.js'; import { AEM_ERROR_CODES, createAEMError, createSuccessResponse, handleAEMHttpError, safeExecute } from './aem.errors.js'; import { CliParams } from '../types.js'; -import { AEMAuth, AEMFetch } from './aem.fetch.js'; +import { AEMFetch } from './aem.fetch.js'; +import { AuthStrategy, OAuthStrategy, createAuthStrategy } from './aem.auth.js'; import { LOGGER } from '../utils/logger.js'; import { exec } from 'child_process'; @@ -10,7 +11,7 @@ export interface AEMConnectorConfig { host: string; author: string; publish: string; - auth: AEMAuth; + authStrategy: AuthStrategy; endpoints: Record; }; mcp: { @@ -42,7 +43,7 @@ export class AEMConnector { this.isAEMaaCS = this.isConfigAEMaaCS(); this.fetch = new AEMFetch({ host: this.config.aem.host, - auth: this.config.aem.auth, + authStrategy: this.config.aem.authStrategy, timeout: this.aemConfig.queries.timeoutMs, }); } @@ -57,33 +58,30 @@ export class AEMConnector { } isConfigAEMaaCS(): boolean { - return Boolean(this.config.aem.auth.clientId && this.config.aem.auth.clientSecret); + return this.config.aem.authStrategy instanceof OAuthStrategy; } loadConfig(params: CliParams = {}): AEMConnectorConfig { - let auth: AEMAuth; - - // OAuth Server-to-Server (client credentials) - if (params.id && params.secret) { - auth = { - clientId: params.id, - clientSecret: params.secret, - }; - } - // Basic Authentication - else { - auth = { - username: params.user || 'admin', - password: params.pass || 'admin', - }; - } - + // Auth-strategy factory chain (feat #5): cert+key > OAuth > Basic. + // Pass all credential candidates; the factory selects and logs a conflict + // warning if cert and OAuth params are both supplied. + const authStrategy: AuthStrategy = createAuthStrategy({ + username: params.user || 'admin', + password: params.pass || 'admin', + clientId: params.id || undefined, + clientSecret: params.secret || undefined, + certPath: params.cert, + keyPath: params.key, + caPath: params.ca, + passphrase: params.passphrase, + }); + return { aem: { host: params.host || 'http://localhost:4502', author: params.host || 'http://localhost:4502', publish: 'http://localhost:4503', - auth, + authStrategy, endpoints: { content: '/content', dam: '/content/dam', diff --git a/src/aem/aem.fetch.ts b/src/aem/aem.fetch.ts index 6411413..dbcde00 100644 --- a/src/aem/aem.fetch.ts +++ b/src/aem/aem.fetch.ts @@ -1,33 +1,11 @@ -import { getAccessToken } from './aem.auth.js'; +import type { Dispatcher } from 'undici'; +import { AuthStrategy } from './aem.auth.js'; import { LOGGER } from '../utils/logger.js'; import { sanitizeUrl, hasUrlCredentials } from '../utils/sanitize.js'; -export type AEMBasicAuth = { - username: string; - password: string; - clientId?: undefined; - clientSecret?: undefined; - accessToken?: undefined; - refreshToken?: undefined; - redirectUri?: undefined; -}; - -export type AEMOAuthServerToServer = { - username?: undefined; - password?: undefined; - clientId: string; - clientSecret: string; - scope?: string | string[]; - accessToken?: undefined; - refreshToken?: undefined; - redirectUri?: undefined; -}; - -export type AEMAuth = AEMBasicAuth | AEMOAuthServerToServer; - export type AEMFetchConfig = { host: string; - auth: AEMAuth; + authStrategy: AuthStrategy; timeout?: number; } @@ -74,125 +52,86 @@ function shouldRetryOn401(response: Response): boolean { export class AEMFetch { private fetch: FetchInstance | null; private readonly config: AEMFetchConfig; - private token: string; - private tokenExpiry: number; - private inflightToken: Promise | null; + private readonly strategy: AuthStrategy; constructor(config: AEMFetchConfig) { if (hasUrlCredentials(config.host)) { - throw new Error('AEM host URL must not contain embedded credentials. Pass them via username/password (Basic) or clientId/clientSecret (OAuth).'); + throw new Error('AEM host URL must not contain embedded credentials. Pass credentials via the AuthStrategy (BasicAuthStrategy / OAuthStrategy / CertAuthStrategy).'); } this.config = config; + this.strategy = config.authStrategy; this.fetch = null; - this.token = ''; - this.tokenExpiry = 0; - this.inflightToken = null; } /** - * Initializes the fetch instance with authentication token. - * Must be called before making requests. + * Initializes the fetch instance. Triggers the strategy's `init()` hook + * once to prime any cached credentials (OAuth: mint IMS token; Basic: noop; + * Cert: read PEM files + build undici.Agent — feat #3). Must be called + * before making requests. */ async init() { - this.token = await this.getAuthToken(this.config.auth); + if (this.strategy.init) { + await this.strategy.init(); + } this.fetch = this.getFetchInstance(); } /** - * True when the configured auth is OAuth Server-to-Server (clientId+secret), - * false when Basic (username+password). Refreshing the token is only useful - * for OAuth — for Basic, re-encoding the same credentials produces the same - * Base64, so a 401-retry is a wasted round-trip. + * Whether a 401 response should trigger a credential refresh + retry. Only + * strategies that implement `refresh()` participate (OAuth mints a new IMS + * token; Basic short-circuits because re-encoding the same credentials + * produces the same Base64; Cert handles rotation via SIGHUP/reload(), not + * 401-driven refresh — see feat #7). */ - private get isOAuth(): boolean { - return !!this.config.auth.clientId && !this.config.auth.username; + private get supportsRefreshOn401(): boolean { + return typeof this.strategy.refresh === 'function'; } /** - * Returns a fetch instance with proper headers for AEM authentication. + * Returns a fetch instance that injects the strategy's headers on every + * request and, for cert-mode, attaches the strategy's `undici.Agent` as the + * fetch dispatcher. */ private getFetchInstance(): FetchInstance { - return (input: RequestInfo, init: RequestInit = {}): Promise => { - // Work with existing headers - create new Headers object to avoid mutating the original - const headers = init.headers instanceof Headers - ? new Headers(init.headers) + return async (input: RequestInfo, init: RequestInit = {}): Promise => { + const headers = init.headers instanceof Headers + ? new Headers(init.headers) : new Headers(init.headers || {}); - - // Always set Authorization (required for all requests) - // Use Bearer for OAuth server-to-server, Basic for username/password - if (this.isOAuth) { - headers.set('Authorization', `Bearer ${this.token}`); - } else { - headers.set('Authorization', `Basic ${this.token}`); + + const authHeaders = await this.strategy.getHeaders(); + for (const [k, v] of Object.entries(authHeaders)) { + headers.set(k, v); } - - // Only set default Accept header if not already set + if (!headers.has('Accept')) { headers.set('Accept', 'application/json'); } - - // Only set default Content-Type if not already set (form data will set it in post()) if (!headers.has('Content-Type')) { headers.set('Content-Type', 'application/json'); } - - // Create new options object with our headers, preserving other init properties - const { headers: _, ...initWithoutHeaders } = init; - return fetch(input, { ...initWithoutHeaders, headers }); - } - } - async getAuthToken(config: AEMAuth): Promise { - // OAuth Server-to-Server (client credentials) - if (config.clientId && config.clientSecret) { - const now = Date.now(); - if (this.token && now < this.tokenExpiry) { - return this.token; - } - // Dedup concurrent mints: if another caller has already kicked off the - // IMS request, ride on its promise instead of issuing a parallel mint. - // Without this, N concurrent post-expiry callers trigger N IMS calls. - if (this.inflightToken) { - return this.inflightToken; + const { headers: _, ...initWithoutHeaders } = init; + const fetchInit: RequestInit = { ...initWithoutHeaders, headers }; + // CertAuthStrategy (feat #3) returns an undici.Dispatcher; Node's + // native fetch accepts it via the `dispatcher` field. `dispatcher` is + // not in the standard RequestInit, so widen the type at this seam. + const dispatcher = this.strategy.getAgent?.(); + if (dispatcher) { + (fetchInit as RequestInit & { dispatcher?: Dispatcher }).dispatcher = dispatcher; } - this.inflightToken = (async () => { - try { - const token = await getAccessToken(config.clientId, config.clientSecret, config.scope); - // Reject expires_in <= 60: a value at-or-below the 60s headroom would place - // tokenExpiry in the past, forcing an IMS mint on every request. Single-flight - // dedups within a tick but still burns a round-trip per call. NaN/undefined - // fail this check too (NaN > 60 is false). - if (!(token.expires_in > 60)) { - throw new Error( - `IMS returned invalid expires_in (${token.expires_in}); must be > 60 seconds to leave refresh headroom.` - ); - } - this.token = token.access_token; - this.tokenExpiry = now + (token.expires_in - 60) * 1000; - return this.token; - } finally { - this.inflightToken = null; - } - })(); - return this.inflightToken; - } - - // Basic Authentication (username/password) - if (config.username && config.password) { - // AEM Sling decodes Basic credentials as ISO-8859-1 (not UTF-8). Encoding the - // source as 'latin1' keeps ASCII identical while making 0x80-0xFF code points - // (é/ü/ñ/etc.) round-trip correctly. Passwords with code points > 0xFF still - // can't be expressed in Basic auth and are out of scope. - return Buffer.from(`${config.username}:${config.password}`, 'latin1').toString('base64'); + return fetch(input, fetchInit); } - - throw new Error('No authentication credentials provided'); } + /** + * Force a credential refresh. Called from `request()` on 401 (gated by + * `supportsRefreshOn401`) and exposed publicly so tests / external callers + * can trigger a refresh without going through a 401. + */ async refreshAuthToken() { - this.token = ''; // Reset token to force refresh - this.tokenExpiry = 0; // Reset expiry - this.token = await this.getAuthToken(this.config.auth); + if (this.strategy.refresh) { + await this.strategy.refresh(); + } } /** * Returns timeout options for fetch requests, including AbortController and timeoutId. @@ -255,7 +194,7 @@ export class AEMFetch { let retryTimeoutId: NodeJS.Timeout | undefined; try { response = await this.fetch(url, options); - if (response.status === 401 && this.isOAuth && shouldRetryOn401(response)) { + if (response.status === 401 && this.supportsRefreshOn401 && shouldRetryOn401(response)) { LOGGER.warn(`AEM request to ${sanitizeUrl(url)} returned 401 Unauthorized. Attempting to refresh token...`); await this.refreshAuthToken(); // Fresh timeout window for the retry: the original signal may already be aborted @@ -310,34 +249,34 @@ export class AEMFetch { error.response = { status: response.status, data: errorText || null }; throw error; } - + // Handle empty responses (common for DELETE operations) // 204 No Content or empty body should return null/empty object if (response.status === 204 || response.status === 200) { const contentType = response.headers.get('content-type') || ''; const contentLength = response.headers.get('content-length'); - + // If it's a DELETE operation and no content, return empty object if (options.method === 'DELETE' && (!contentLength || contentLength === '0')) { return {}; } - + // If content-type is not JSON and no content, return empty object if (!contentType.includes('application/json') && (!contentLength || contentLength === '0')) { return {}; } } - + if (isHtml) { return response.text(); } - + // Check if response has content before parsing JSON const text = await response.text(); if (!text || text.trim().length === 0) { return {}; } - + // Try to parse as JSON, but handle non-JSON responses gracefully try { return JSON.parse(text); @@ -387,7 +326,7 @@ export class AEMFetch { const headers = options.headers instanceof Headers ? new Headers(options.headers) : new Headers(options.headers || {}); - + if (data instanceof URLSearchParams) { body = data; // Set Content-Type for form data - this must be set explicitly @@ -399,7 +338,7 @@ export class AEMFetch { headers.set('Content-Type', 'application/json'); } } - + const fullUrl = this.buildUrlWithParams(url); // Remove headers from options to avoid conflicts, then set our merged headers const { headers: _, ...optionsWithoutHeaders } = options; @@ -431,12 +370,12 @@ export class AEMFetch { if (!this.fetch) { throw new Error('AEMFetch not initialized. Call await init() before making requests.'); } - + let body: BodyInit; const headers = options.headers instanceof Headers ? new Headers(options.headers) : new Headers(options.headers || {}); - + if (data instanceof URLSearchParams) { body = data; headers.set('Content-Type', 'application/x-www-form-urlencoded'); @@ -446,7 +385,7 @@ export class AEMFetch { headers.set('Content-Type', 'application/json'); } } - + const fullUrl = this.buildUrlWithParams(url); const { timeoutId, signal } = this.getTimeoutOptions(timeout); if (timeout) { @@ -462,7 +401,7 @@ export class AEMFetch { headers }); - if (response.status === 401 && this.isOAuth && shouldRetryOn401(response)) { + if (response.status === 401 && this.supportsRefreshOn401 && shouldRetryOn401(response)) { await this.refreshAuthToken(); // Fresh timeout window for the retry: the original signal may already be aborted // if the refresh took longer than the original `timeout`. diff --git a/src/cli.ts b/src/cli.ts index b757320..5f1071f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,6 +5,7 @@ import { hideBin } from 'yargs/helpers'; import { startServer } from './index.js'; import { CliParams } from './types'; import { hasUrlCredentials } from './utils/sanitize.js'; +import { CertParamsSchema } from './aem/aem.auth.schemas.js'; type CliArgs = CliParams & { help?: boolean; @@ -16,6 +17,25 @@ const argv: CliArgs = yargs(hideBin(process.argv)).options({ pass: { type: 'string', default: 'admin', alias: 'p' }, id: { type: 'string', default: '', alias: 'i', describe: 'clientId' }, secret: { type: 'string', default: '', alias: 's', describe: 'clientSecret' }, + cert: { + type: 'string', + alias: 'C', + describe: 'path to client certificate PEM file for mTLS to AEM. Env: AEM_CERT_PATH.', + }, + key: { + type: 'string', + alias: 'k', + describe: 'path to private key PEM file for mTLS to AEM. Env: AEM_KEY_PATH.', + }, + ca: { + type: 'string', + describe: 'path to CA bundle PEM file (only needed for self-signed AEM tenants). Env: AEM_CA_PATH.', + }, + 'cert-watch-interval-min': { + type: 'number', + default: Number(process.env.AEM_CERT_WATCH_INTERVAL_MIN) || 0, + describe: 'periodically check the cert file mtime every N minutes; on change, reload PEMs and rebuild the undici.Agent (rotation without restart). 0 disables (default). SIGHUP still works regardless. Env: AEM_CERT_WATCH_INTERVAL_MIN.', + }, mcpPort: { type: 'number', default: 8502, alias: 'm' }, bind: { type: 'string', @@ -51,4 +71,46 @@ if (host && hasUrlCredentials(host)) { process.exit(1); } -startServer({ host, user, pass, mcpPort, id, secret, allowOrigin, bind, shutdownDrainSeconds }); +// Cert-auth params. Flags take precedence over env vars; `??` only falls +// through on null/undefined so an empty `--cert ""` reaches the schema and +// gets rejected by `.min(1)` (instead of being silently coerced to "no cert +// provided"). `passphrase` is env-only — no CLI flag — to keep the secret +// out of `ps aux`. +const certInput = { + cert: argv.cert ?? process.env.AEM_CERT_PATH ?? undefined, + key: argv.key ?? process.env.AEM_KEY_PATH ?? undefined, + ca: argv.ca ?? process.env.AEM_CA_PATH ?? undefined, + passphrase: process.env.AEM_KEY_PASSPHRASE || undefined, +}; + +const certValidation = CertParamsSchema.safeParse(certInput); +if (!certValidation.success) { + // One-line sanitized error. NEVER echo `issue.received` — a malformed + // passphrase value would leak into stderr / CI logs. + const issue = certValidation.error.issues[0]; + const pathSeg = issue?.path?.[0]; + const label = typeof pathSeg === 'string' && pathSeg.length > 0 ? `--${pathSeg}` : 'cert-auth'; + console.error(`Error: ${label}: ${issue?.message ?? 'validation failed'}`); + process.exit(1); +} + +const { cert, key, ca, passphrase } = certValidation.data; + +const certWatchIntervalMin = argv.certWatchIntervalMin ?? 0; + +startServer({ + host, + user, + pass, + mcpPort, + id, + secret, + cert, + key, + ca, + passphrase, + certWatchIntervalMin, + allowOrigin, + bind, + shutdownDrainSeconds, +}); diff --git a/src/server/app.server.ts b/src/server/app.server.ts index c4f1d7b..93d5c6f 100644 --- a/src/server/app.server.ts +++ b/src/server/app.server.ts @@ -1,13 +1,22 @@ +import fs from 'node:fs'; import express, { Request, Response, NextFunction } from 'express'; import cors from 'cors'; import { handleRequest } from '../mcp/mcp.server-handler.js'; // import { useBasicAuth } from './app.auth.js'; import { AEMConnector } from '../aem/aem.connector.js'; +import { destroyAllCertStrategies, reloadAllCertStrategies } from '../aem/aem.auth.js'; import { config } from '../config.js'; import { CliParams } from '../types.js'; import { LOGGER } from '../utils/logger.js'; import { transports } from '../mcp/mcp.transports.js'; +// Cap on how long we wait for `destroyAllCertStrategies()` during shutdown +// (feat #6). The undici Agent's `destroy()` is normally near-instant — it +// just closes the keep-alive socket pool — but a stuck socket or hostile +// peer could otherwise hang the process past the drain deadline. 5s is plenty +// for real shutdowns and short enough to keep `kill -INT` responsive. +const CERT_DESTROY_TIMEOUT_MS = 5_000; + // MCP spec MUST: validate Origin header to prevent DNS-rebinding attacks. // Defaults cover the official MCP Inspector (UI :6274, proxy :6277) on both // loopback hostnames. Extra origins via --allow-origin CLI flag or comma- @@ -172,12 +181,36 @@ export const startServer = (params: CliParams = {}) => { // resolves and the process is otherwise idle, exit cleanly. forceExit.unref(); - server.close((err) => { + server.close(async (err) => { clearTimeout(forceExit); const elapsed = ((Date.now() - startedAt) / 1000).toFixed(2); if (err) { process.stderr.write(`[shutdown] server.close error: ${err.message}\n`); } + // Cert-mode hook (feat #6): release any cached undici.Agent keep-alive + // socket pools so they don't linger past process.exit. No-op when the + // active strategies are Basic/OAuth (registry empty → count 0). Bounded + // by CERT_DESTROY_TIMEOUT_MS so a stuck Agent can't hang the process. + try { + const destroyed = await Promise.race([ + destroyAllCertStrategies(), + new Promise((_, reject) => + setTimeout( + () => reject(new Error(`cert destroy timeout (${CERT_DESTROY_TIMEOUT_MS}ms)`)), + CERT_DESTROY_TIMEOUT_MS + ) + ), + ]); + if (destroyed > 0) { + process.stderr.write( + `[shutdown] destroyed ${destroyed} cert-auth agent pool(s)\n` + ); + } + } catch (e: any) { + process.stderr.write( + `[shutdown] cert-auth destroy error: ${e?.message ?? e}\n` + ); + } process.stderr.write(`[shutdown] drain complete in ${elapsed}s — exit 0\n`); process.exit(0); }); @@ -206,6 +239,75 @@ export const startServer = (params: CliParams = {}) => { process.on('SIGINT', () => drain('SIGINT')); process.on('SIGTERM', () => drain('SIGTERM')); + // Cert rotation hook (feat #7). SIGHUP triggers a reload of every live + // CertAuthStrategy: re-read PEMs, atomic Agent swap, 30s drain on the old + // Agent. Stderr-only (unconditional) so SREs see the transition without + // needing MCP_LOGGER. No-op when no cert-mode strategy is active. + const onSighup = async () => { + if (shuttingDown) return; + process.stderr.write('[cert-reload] SIGHUP received — reloading cert-auth strategies\n'); + try { + const { reloaded, errors } = await reloadAllCertStrategies(); + if (reloaded === 0 && errors.length === 0) { + process.stderr.write('[cert-reload] no cert-auth strategies live; nothing to reload\n'); + } else if (reloaded > 0) { + process.stderr.write(`[cert-reload] reloaded ${reloaded} cert-auth strategy(ies)\n`); + } + for (const err of errors) { + process.stderr.write(`[cert-reload] error: ${err}\n`); + } + } catch (e: any) { + process.stderr.write(`[cert-reload] fatal error: ${e?.message ?? e}\n`); + } + }; + process.on('SIGHUP', () => { void onSighup(); }); + + // Optional mtime polling. When --cert-watch-interval-min N is non-zero AND + // a cert path was supplied, poll cert mtime every N minutes; on change, + // trigger the same reload flow as SIGHUP. setInterval.unref() so the timer + // alone doesn't keep the process alive on shutdown. + const watchMinutes = params?.certWatchIntervalMin ?? 0; + const certPath = params?.cert; + if (watchMinutes > 0 && certPath) { + let lastMtimeMs: number | undefined; + try { + lastMtimeMs = fs.statSync(certPath).mtimeMs; + } catch { + // The cert path was already validated by CertAuthStrategy.init() at + // boot; a stat failure here is unusual. Log and skip the watcher + // rather than returning early — fatal-error fallbacks below must still + // be registered regardless of watcher setup. + process.stderr.write(`[cert-watch] cannot stat cert path at boot — watcher disabled\n`); + } + if (lastMtimeMs !== undefined) { + const intervalMs = watchMinutes * 60_000; + process.stderr.write( + `[cert-watch] watching cert mtime every ${watchMinutes} minute(s)\n` + ); + // NOTE: only cert mtime is watched. Key and CA file changes are not + // detected by this poller — use SIGHUP to force reload when rotating key or CA. + const watchTimer = setInterval(async () => { + if (shuttingDown) return; + let currentMtimeMs: number; + try { + currentMtimeMs = fs.statSync(certPath).mtimeMs; + } catch (e: any) { + process.stderr.write(`[cert-watch] stat error: ${e?.message ?? e}\n`); + return; + } + if (currentMtimeMs !== lastMtimeMs) { + process.stderr.write( + `[cert-watch] cert mtime changed (was ${new Date(lastMtimeMs!).toISOString()}, ` + + `now ${new Date(currentMtimeMs).toISOString()}) — reloading\n` + ); + lastMtimeMs = currentMtimeMs; + await onSighup(); + } + }, intervalMs); + watchTimer.unref(); + } + } + // Fatal-error fallbacks. Node docs are explicit that the process is in an // undefined state after `uncaughtException` — we MUST NOT try to resume // normal work or run the full async drain. Do sync-only cleanup (close diff --git a/src/test/smoke-feat-5.mjs b/src/test/smoke-feat-5.mjs new file mode 100644 index 0000000..ed0aadd --- /dev/null +++ b/src/test/smoke-feat-5.mjs @@ -0,0 +1,227 @@ +// Auth factory selection smoke test for feat #5. +// +// Covers test cases from `plan/mcp-cert-auth-implementation.md` § feat #5: +// - cert+key → CertAuthStrategy (highest priority) +// - id+secret → OAuthStrategy +// - user+pass → BasicAuthStrategy +// - cert+key + id+secret → CertAuthStrategy + conflict warning logged +// - partial OAuth (only id, no secret) with cert+key → also warns +// +// The conflict warning now uses process.stderr.write unconditionally (no longer +// gated by MCP_LOGGER) so it is visible in default deployments. We capture +// stderr.write to intercept these lines without needing MCP_LOGGER at all. +// +// Run: +// npm run build && node src/test/smoke-feat-5.mjs + +const warnings = []; +const origStderrWrite = process.stderr.write.bind(process.stderr); +process.stderr.write = (chunk, ...rest) => { + warnings.push(typeof chunk === 'string' ? chunk : chunk.toString()); + return origStderrWrite(chunk, ...rest); +}; + +const { createAuthStrategy, BasicAuthStrategy, OAuthStrategy, CertAuthStrategy } = + await import('../../dist/aem/aem.auth.js'); + +const pass = []; +const fail = []; +const check = (cond, msg) => (cond ? pass : fail).push(msg); + +const clearWarnings = () => { warnings.length = 0; }; + +// ============================================================ +// (1) user+pass only → BasicAuthStrategy +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ username: 'admin', password: 'admin' }); + check(s instanceof BasicAuthStrategy, + `(1) user+pass → BasicAuthStrategy (got ${s.constructor.name})`); + check(warnings.length === 0, + `(1) no warning emitted for Basic-only (got ${warnings.length} warnings)`); +} + +// ============================================================ +// (2) id+secret only → OAuthStrategy +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ clientId: 'cid', clientSecret: 'csec' }); + check(s instanceof OAuthStrategy, + `(2) id+secret → OAuthStrategy (got ${s.constructor.name})`); + check(warnings.length === 0, + `(2) no warning emitted for OAuth-only`); +} + +// ============================================================ +// (3) cert+key only → CertAuthStrategy, no warning +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ certPath: '/tmp/c.crt', keyPath: '/tmp/c.key' }); + check(s instanceof CertAuthStrategy, + `(3) cert+key → CertAuthStrategy (got ${s.constructor.name})`); + check(warnings.length === 0, + `(3) no warning emitted for cert-only`); +} + +// ============================================================ +// (4) cert+key + id+secret → CertAuthStrategy, conflict warning +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ + certPath: '/tmp/c.crt', keyPath: '/tmp/c.key', + clientId: 'cid', clientSecret: 'csec', + }); + check(s instanceof CertAuthStrategy, + `(4) cert+key + OAuth → CertAuthStrategy wins (got ${s.constructor.name})`); + const joined = warnings.join(' | '); + const mentionsPriority = /priority|precedence/i.test(joined); + const mentionsIgnored = /ignored/i.test(joined); + const mentionsCert = /cert/i.test(joined); + const mentionsOAuth = /OAuth/i.test(joined); + check(warnings.length >= 1 && mentionsPriority && mentionsIgnored && mentionsCert && mentionsOAuth, + `(4) conflict warning logged with priority/ignored/cert/OAuth language (count=${warnings.length}, msg="${joined}")`); +} + +// ============================================================ +// (5) cert+key + partial OAuth (only id, no secret) → also warns +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ + certPath: '/tmp/c.crt', keyPath: '/tmp/c.key', + clientId: 'cid', // no secret + }); + check(s instanceof CertAuthStrategy, + `(5) cert+key + partial OAuth → CertAuthStrategy`); + check(warnings.length >= 1, + `(5) partial OAuth (id only) still triggers conflict warning (got ${warnings.length})`); +} +{ + clearWarnings(); + const s = createAuthStrategy({ + certPath: '/tmp/c.crt', keyPath: '/tmp/c.key', + clientSecret: 'csec', // no id + }); + check(s instanceof CertAuthStrategy, + `(5b) cert+key + partial OAuth (secret only) → CertAuthStrategy`); + check(warnings.length >= 1, + `(5b) partial OAuth (secret only) still triggers conflict warning`); +} + +// ============================================================ +// (6) cert+key + user+pass (non-default) → CertAuthStrategy, NO warning +// (Basic always defaults to admin/admin in CLI, we can't distinguish +// explicit-default from "user didn't pass" — silent precedence is correct) +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ + certPath: '/tmp/c.crt', keyPath: '/tmp/c.key', + username: 'someone', password: 'else', + }); + check(s instanceof CertAuthStrategy, + `(6) cert+key + user+pass → CertAuthStrategy`); + check(warnings.length === 0, + `(6) no warning emitted for cert + Basic (plan: only cert-vs-OAuth warns)`); +} + +// ============================================================ +// (7) No credentials → throws +// ============================================================ +{ + let threw = false; + let msg = ''; + try { createAuthStrategy({}); } catch (e) { threw = true; msg = e.message; } + check(threw && /no authentication/i.test(msg), + `(7) empty input throws "no authentication credentials" (got: "${msg}")`); +} + +// ============================================================ +// (8) Cert without key → falls through (not handled by factory; CLI catches via Zod) +// ============================================================ +{ + clearWarnings(); + const s = createAuthStrategy({ + certPath: '/tmp/c.crt', // no keyPath + username: 'admin', password: 'admin', + }); + check(s instanceof BasicAuthStrategy, + `(8) cert without key → falls through to Basic (Zod catches this at CLI; factory just doesn't pick cert)`); +} + +// ============================================================ +// (9) Cert params (ca + passphrase) pass through to CertAuthStrategy +// ============================================================ +{ + const s = createAuthStrategy({ + certPath: '/tmp/c.crt', keyPath: '/tmp/c.key', + caPath: '/tmp/ca.crt', passphrase: 'secret', + }); + check(s instanceof CertAuthStrategy, + `(9) cert+key+ca+passphrase → CertAuthStrategy`); + // Tested via private `params` would be invasive; the fact that init() works + // with passphrase is already covered by smoke-cert-auth.mjs (f.2). Here we + // only verify the factory routed through. +} + +process.stderr.write = origStderrWrite; + +// ============================================================ +// (10) redactCliParams correctly identifies cert mode (log fidelity) +// Bug fixed in feat #5: sanitize used to fall back to 'basic' when +// cert/key were supplied because it never inspected those fields. +// ============================================================ +{ + const { redactCliParams } = await import('../../dist/utils/sanitize.js'); + { + const r = redactCliParams({ host: 'https://aem.example.com', user: 'admin', pass: 'admin' }); + check(r.authMode === 'basic', `(10a) user+pass → authMode 'basic' (got '${r.authMode}')`); + } + { + const r = redactCliParams({ host: 'https://aem.example.com', id: 'cid', secret: 'csec' }); + check(r.authMode === 'oauth', `(10b) id+secret → authMode 'oauth' (got '${r.authMode}')`); + } + { + const r = redactCliParams({ + host: 'https://aem.example.com', + user: 'admin', pass: 'admin', + cert: '/tmp/c.crt', key: '/tmp/c.key', ca: '/tmp/ca.crt', + }); + check(r.authMode === 'cert', + `(10c) cert+key (with default user+pass) → authMode 'cert' (got '${r.authMode}')`); + check(r.hasCert && r.hasKey && r.hasCa, + `(10c) hasCert/hasKey/hasCa surfaced (got cert=${r.hasCert} key=${r.hasKey} ca=${r.hasCa})`); + } + { + const r = redactCliParams({ + host: 'https://aem.example.com', + cert: '/tmp/c.crt', key: '/tmp/c.key', + passphrase: 'super-secret', + }); + check(r.hasPassphrase === true, + `(10d) hasPassphrase = true when passphrase supplied`); + // Make sure the passphrase value itself is NEVER in the output + const dump = JSON.stringify(r); + check(!dump.includes('super-secret'), + `(10d) redacted output does NOT contain the passphrase value (defense-in-depth)`); + } + { + const r = redactCliParams({ host: 'https://aem.example.com' }); + check(r.authMode === 'none', `(10e) no creds → authMode 'none' (got '${r.authMode}')`); + } +} + +console.log(''); +console.log(`PASS (${pass.length}):`); +pass.forEach((p) => console.log(` PASS ${p}`)); +if (fail.length > 0) { + console.log(`FAIL (${fail.length}):`); + fail.forEach((f) => console.log(` FAIL ${f}`)); +} +console.log(''); +console.log(`Result: ${fail.length === 0 ? 'ALL PASS' : `${fail.length} FAILED`}`); +process.exit(fail.length === 0 ? 0 : 1); diff --git a/src/types.ts b/src/types.ts index c4041a6..4984dd1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,6 +4,11 @@ export type CliParams = { pass?: string; id?: string; secret?: string; + cert?: string; + key?: string; + ca?: string; + passphrase?: string; + certWatchIntervalMin?: number; mcpPort?: number; allowOrigin?: string[]; bind?: string; diff --git a/src/utils/sanitize.ts b/src/utils/sanitize.ts index 4911600..7b9eefd 100644 --- a/src/utils/sanitize.ts +++ b/src/utils/sanitize.ts @@ -95,18 +95,23 @@ function truncate(s: string, maxLen: number): string { export type RedactedCliParams = { host: string; - authMode: 'basic' | 'oauth' | 'none'; + authMode: 'cert' | 'basic' | 'oauth' | 'none'; mcpPort?: number; hasUser: boolean; hasPass: boolean; hasId: boolean; hasSecret: boolean; + hasCert: boolean; + hasKey: boolean; + hasCa: boolean; + hasPassphrase: boolean; }; /** * Render `CliParams` in a form safe to log. Strips userinfo from `host`, * collapses credential presence to booleans, and surfaces the auth mode - * without echoing any secret value. + * without echoing any secret value. Selection order matches + * `createAuthStrategy` (feat #5): cert+key > id+secret > user+pass. */ export function redactCliParams(p: { host?: string; @@ -114,14 +119,25 @@ export function redactCliParams(p: { pass?: string; id?: string; secret?: string; + cert?: string; + key?: string; + ca?: string; + passphrase?: string; mcpPort?: number; }): RedactedCliParams { const hasUser = !!p.user; const hasPass = !!p.pass; const hasId = !!p.id; const hasSecret = !!p.secret; + const hasCert = !!p.cert; + const hasKey = !!p.key; + const hasCa = !!p.ca; + const hasPassphrase = !!p.passphrase; const authMode: RedactedCliParams['authMode'] = - hasId && hasSecret ? 'oauth' : hasUser && hasPass ? 'basic' : 'none'; + hasCert && hasKey ? 'cert' + : hasId && hasSecret ? 'oauth' + : hasUser && hasPass ? 'basic' + : 'none'; return { host: p.host ? sanitizeUrl(p.host) : '', authMode, @@ -130,5 +146,9 @@ export function redactCliParams(p: { hasPass, hasId, hasSecret, + hasCert, + hasKey, + hasCa, + hasPassphrase, }; }