Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3e4e432
build(dependencies): update package-lock.json via npm audit fix
Jun 10, 2026
9038ccf
docs: add CLAUDE.md for project guidance and command reference
Jun 10, 2026
a9a1a59
fix: leak #1 - sanitize URLs and reject embedded credentials in host
Jun 10, 2026
f586da6
fix: leak #2 - redact CLI params before logging session init
Jun 10, 2026
f6f12db
fix: leak #1b - sanitize URLs in error messages
Jun 10, 2026
18d4780
fix: leak #3 - strip Authorization on cross-origin redirect
Jun 10, 2026
11e4964
fix: leak #4 - summarize AEM response bodies in error details
Jun 10, 2026
fe6ad4e
fix: leak #5 - deduplicate concurrent OAuth token mints
Jun 10, 2026
77427a6
fix: leak #6 - use fresh AbortSignal on 401 retry
Jun 10, 2026
016776b
fix: leak #7 - skip 401 retry on permission errors
Jun 10, 2026
ee30674
fix: leak #8 - encode Basic auth credentials as Latin-1
Jun 10, 2026
6c2c571
fix: leak #9 - skip 401 retry for Basic auth
Jun 10, 2026
a647b0c
fix: leak #10 - allow IMS endpoint override via AEM_IMS_URL
Jun 10, 2026
074095b
build(gitignore): exclude .DS_Store, src/test/, and plan/
nc-markovic Jun 11, 2026
c1dcc36
fix: leak #14 - reject expires_in <= 60 to prevent IMS storm
nc-markovic Jun 11, 2026
3216fc5
fix: leak #15 - validate Origin header to prevent DNS rebinding
nc-markovic Jun 11, 2026
0f36232
docs: codify testing workflow for plan-driven work in CLAUDE.md
nc-markovic Jun 11, 2026
be4cb2e
fix: leak #16 - bind to loopback by default to prevent LAN exposure
nc-markovic Jun 11, 2026
d5eab21
fix: leak #17 - graceful drain on SIGINT/SIGTERM + fatal-error fallbacks
nc-markovic Jun 11, 2026
c4b710b
docs: codify "clarify before deciding" and testing workflow in CLAUDE.md
nc-markovic Jun 11, 2026
aedd490
fix: leak #18 - stale session returns 404 to enable Inspector auto-re…
nc-markovic Jun 11, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,13 @@ dist
*.swo
*~

# macOS
.DS_Store
**/.DS_Store

# Local mTLS test certificates (throwaway, generated by src/test/gen-test-certs.sh)
src/test/

# AEM connection and auth files
.aem-connection.json
.aem-oauth-tokens.json
Expand All @@ -170,3 +177,5 @@ dist
TEST_RESULTS.md
INTERACTIVE_TESTING.md
comprehensive-test.js

plan/
108 changes: 108 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# CLAUDE.md

This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.

## Commands

```sh
npm run build # build:types (tsc, .d.ts only) + build:ts (esbuild bundle to dist/)
npm run build:ts # esbuild only — fastest iteration when types are unchanged
npm run build:types # tsc --emitDeclarationOnly (also doubles as the typecheck)
npm start # node dist/cli.js with MCP_LOGGER=true
npm run test:dev # node ./dist/cli.js (no logger)
npm run test:npm # pack + global install + smoke-run on port 5502
```

No test runner, linter, or formatter is configured. `npm run build:types` is the only available typecheck.

Single-process run with custom params (mirrors the `aem-mcp` bin):
```sh
node dist/cli.js -H=https://author.example.com -u=user -p=pass # Basic auth
node dist/cli.js -H=https://author.example.com -i=<clientId> -s=<clientSecret> # AEMaaCS OAuth S2S
MCP_USERNAME=foo MCP_PASSWORD=bar node dist/cli.js # gate /mcp with HTTP Basic
```

`MCP_LOGGER=true` enables logging. Without it, `LOGGER` is a no-op — this is intentional (stdout must stay clean for MCP stdio clients; see `src/utils/logger.ts`).

## Clarify before deciding

Before taking any action that involves a real choice — credentials, destructive
vs read-only, which approach to take, which environment to touch, what scope
to cover — **ask the user first** with concrete options (use the
AskUserQuestion tool when it's a short choice). Do not infer credentials,
assume "the obvious" tool from a list, or default to a destructive path
because it's listed in a plan.

What counts as "a real choice":
- Anything that mutates external state (a real AEM, a real database, a real
remote — even via a read-only-looking tool that might be misconfigured).
- Anything where multiple plausible paths exist and the safe / fast / cheap
trade-off is not obvious from context.
- Anything where the user hasn't given an explicit instruction in *this*
conversation and there's no durable preference in memory.

What does NOT count: typechecking, running local terminal-only tests, reading
files, writing to gitignored harnesses, recompiling. Take those without
asking.

## Testing workflow

When working on any item that has a `How to Test` block (e.g. each entry in
`plan/mcp-cert-auth-implementation.md`), **run every test you can run from the
terminal yourself**. That includes ad-hoc Node harnesses, `curl` against a
locally-started server, port checks, etc. The repo has no test runner — write a
one-off `.mjs` under `src/test/` when needed and run it.

Show the results back to the user. Only ask the user to run a test manually
when you genuinely cannot execute it from this session (e.g. browser-driven
flows like the MCP Inspector UI, real-IMS / real-AEM tenants, OS-level signals
across machines). When you do hand off, give **exact step-by-step commands**
the user can paste — never "please verify X" without the recipe.

Typecheck (`npm run build:types`) is necessary but not sufficient. Plan-
specified runtime behaviour must actually be observed.

## Architecture

The server is a Model Context Protocol (MCP) gateway that translates JSON-RPC tool calls into AEM HTTP operations (Sling/QueryBuilder/JCR). Request flow:

```
MCP client ──HTTP POST /mcp──▶ Express (server/app.server.ts)
server-handler.ts ──per-session──▶ StreamableHTTPServerTransport
│ │
▼ ▼
mcp.server.ts (MCP SDK Server) transports map (mcp.transports.ts)
│ CallToolRequest
mcp.aem-handler.ts (switch on method name)
aem.connector.ts ──▶ aem.fetch.ts ──HTTP──▶ AEM
```

Key seams:

- **`src/cli.ts`** — yargs entry. Parses `--host/--user/--pass/--id/--secret/--mcpPort` into `CliParams` (see `src/types.ts`) and calls `startServer`.
- **`src/server/app.server.ts`** — Express app. Exposes `GET /` (info), `GET /health`, `POST /mcp`. `GET/DELETE /mcp` return 405. CORS is wide open (`origin: '*'`). Optional Basic auth middleware exists in `app.auth.ts` but is **currently commented out** at the `useBasicAuth(app)` line — re-enable by uncommenting both the import and call. The middleware only activates if both `MCP_USERNAME` and `MCP_PASSWORD` env vars are set.
- **`src/mcp/mcp.server-handler.ts`** — Per-request session routing. New `StreamableHTTPServerTransport` is created on `initialize`; subsequent requests reuse the transport by `mcp-session-id` header. A new `MCPRequestHandler` (and therefore a new `AEMConnector`) is constructed per session — there's no global AEM client.
- **`src/mcp/mcp.server.ts`** — Wires MCP SDK handlers: `ListTools` returns the static `tools` array; `CallTool` dispatches via `MCPRequestHandler.handleRequest`. Has special-case handling for `OAUTH_REQUIRED` errors that surface an `authUrl` back to the client.
- **`src/mcp/mcp.tools.ts`** — The single source of truth for the MCP tool surface (~46 tools, JSON-Schema input definitions). When adding a tool you must update **both** this file (schema) and `mcp.aem-handler.ts` (the switch statement that routes `method` → `AEMConnector.xxx`). They are not auto-derived from each other.
- **`src/aem/aem.connector.ts`** — The big file (~3700 lines). All AEM domain logic — pages, components, assets, workflows, replication, search. Every public method wraps its body in `safeExecute` (from `aem.errors.ts`) and returns either `createSuccessResponse(...)` or throws an `AEMOperationError` with a code from `AEM_ERROR_CODES`. Path inputs are validated against `isValidContentPath` (must start with `/content`, `/content/dam`, `/conf`, or `/content/experience-fragments` — see `aem.config.ts`).
- **`src/aem/aem.fetch.ts`** — Thin fetch wrapper. Two auth modes selected by `loadConfig` in the connector: Basic (`user`/`pass`) sends `Authorization: Basic <b64>`; OAuth S2S (`id`/`secret`) calls Adobe IMS (`aem.auth.ts`) to mint a Bearer token, caches it with `expires_in - 60s` headroom, and retries once on 401 by refreshing the token. Form posts use `URLSearchParams` and set `Content-Type: application/x-www-form-urlencoded` (required for SlingPostServlet).

## Component update conventions

Two behaviors are non-obvious and worth knowing before touching component code paths:

1. **Dialog-driven property validation.** `updateComponent` and `addComponent` fetch the component's `cq:dialog` (and walk `sling:resourceSuperType` recursively) to build a `fieldDefinitions` map, then validate provided properties against it — select fields check against the dialog's option values, checkboxes against boolean coercion, numberfields against `Number(...)`. See `getComponentDefinition` and `validateComponentProperties` in `aem.connector.ts`.
2. **`cq:template` materialization.** When `addComponent` finds a `cq:template` node under the component definition (e.g., column controls), it merges template properties into the new node and creates the template's child nodes via separate POSTs. See `getComponentTemplate` and `applyTemplateChildNodes`.

## Important constraints

- **ESM-only** (`"type": "module"` in `package.json`). Internal imports use `.js` suffixes even in `.ts` files — this is required, not a mistake.
- **AGPL-3.0** license — avoid copying code from sources incompatible with this license.
- Releases are driven by **semantic-release** parsing Angular-style commits (`feat:`, `fix:`, `BREAKING CHANGE:` footer); see `docs/CONTRIBUTING.md`. Do not bump versions manually.
- `executeJCRQuery` is **not** a JCR SQL2 executor despite the name — it's a thin wrapper around QueryBuilder fulltext (see the docstring on the method). Don't change its behavior to actually run SQL2 without coordinating with callers.
- The `src/test/` directory contains certificate-generation and mTLS test-server scripts for in-progress cert-auth work, not unit tests.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ Options:
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`.

#### Environment variables

| Variable | Purpose |
|---|---|
| `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). |
| `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. |

### Example Command
```sh
aem-mcp -u=user@domain.com -p=mypass -H=https://author-qa.domain.com
Expand Down
65 changes: 65 additions & 0 deletions docs/phase1-smoke-audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# 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 '{...}'
```
Loading
Loading