Skip to content

Commit d4589db

Browse files
committed
harden: validate containment depth, loopback range, and test coverage
Add realpath-safe validate reads, broken-symlink rejection, 127.0.0.0/8 loopback token policy, expanded DML/format tests, and consumer-surface parity.
1 parent b261df8 commit d4589db

17 files changed

Lines changed: 433 additions & 130 deletions

.changeset/read-surface-hardening.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"@stainless-code/codemap": patch
33
---
44

5-
Harden read surfaces: `codemap query --format …` blocks index mutations via the same read-only guard as `--json`; `codemap serve` requires `--token` when `--host` is not loopback; `codemap validate` (and MCP/HTTP `validate`) can return `rejected` rows with a `reason` when a path escapes the project root or resolves outside via symlink.
5+
Harden read surfaces: `codemap query --format …` blocks index mutations via the same read-only guard as `--json`; `codemap serve` requires `--token` when `--host` is not loopback (any `127.0.0.0/8` address counts as loopback, so `--token` stays optional on `127.0.0.2` and similar); `codemap validate` (and MCP/HTTP `validate`) can return `rejected` rows with a `reason` when a path escapes the project root, resolves outside via symlink, or is a broken symlink — output `path` keys are always project-relative POSIX paths.

docs/architecture.md

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

docs/glossary.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ CI-aggregate flag on `codemap query` and `codemap audit`. Aliases `--format sari
125125

126126
### `codemap validate`
127127

128-
CLI subcommand comparing on-disk SHA-256 against `files.content_hash`. Statuses: `stale | missing | unindexed | rejected` (`rejected` carries optional `reason` when a path escapes the project root or resolves outside via symlink). Exits `1` on any drift.
128+
CLI subcommand comparing on-disk SHA-256 against `files.content_hash`. Statuses: `stale | missing | unindexed | rejected` (`rejected` carries optional `reason` when a path escapes the project root, resolves outside via symlink, or has a broken symlink; output `path` keys are always project-relative POSIX paths). Exits `1` on any drift.
129129

130130
### `module_cycles` (table) / circular imports
131131

@@ -565,7 +565,7 @@ Long-running process that subscribes to filesystem changes via [chokidar v5](htt
565565

566566
### `codemap serve` / HTTP server
567567

568-
Long-running HTTP server exposing the same tool taxonomy as `codemap mcp` over `POST /tool/{name}` for non-MCP consumers (CI scripts, simple `curl`, IDE plugins that don't speak MCP). Default bind **`127.0.0.1:7878`** (loopback only — refuse `0.0.0.0` unless explicitly opted in via `--host 0.0.0.0`); `--token <secret>` is optional on loopback binds and **required** on non-loopback binds — when set, every request needs `Authorization: Bearer <secret>`. HTTP returns each tool's native JSON payload directly (NOT MCP's `{content: [...]}` wrapper); `query` / `query_recipe` match `codemap query --json` row arrays unless `summary` / `group_by` reshape the envelope, or `baseline` returns a diff envelope (incompatible with non-`json` `format` / `group_by`; save/list/drop remain separate tools); parity twins (`query batch`, `trace`, `explore`, `node`, `file`, `schema`, `symbols`, `context`, `ingest-coverage`, `ingest-churn`) always emit JSON on CLI without `--json`; other tools match their CLI `--json` payloads when that flag is set; `format: "sarif"` payloads ship as `application/sarif+json`, `format: "annotations"` / `"mermaid"` / `"diff"` / `"badge"` (markdown) as `text/plain; charset=utf-8`, `format: "diff-json"` / `"codeclimate"` / `"badge"` + `badge_style: "json"` as `application/json; charset=utf-8`. Routes: `POST /tool/{name}` (every MCP tool), `GET /resources/{encoded-uri}` (resource handler for `codemap://recipes`, `codemap://recipes/{id}`, `codemap://schema`, `codemap://skill`, `codemap://rule`, `codemap://mcp-instructions`, `codemap://files/{path}`, and `codemap://symbols/{name}`), `GET /health` (auth-exempt liveness probe — does not start the watcher), `GET /tools` / `GET /resources` (catalogs). With `--watch`, chokidar is refcount-gated per request and stops 5s after the last client (`HTTP_WATCH_RELEASE_GRACE_MS`) — distinct from MCP idle shutdown; the HTTP process keeps listening. Pure transport — same `tool-handlers.ts` / `resource-handlers.ts` MCP uses; no engine duplication. Errors → `{"error": "..."}` with HTTP status 400 / 401 / 403 / 404 / 500. SIGINT / SIGTERM → graceful drain. Every response carries `X-Codemap-Version: <semver>`. **CSRF + DNS-rebinding guard:** every request (including auth-exempt `/health`) is evaluated against `Sec-Fetch-Site` / `Origin` / `Host` when present — modern browsers send `Sec-Fetch-Site` and `Origin` on cross-origin fetches (header presence varies by request type, browser, and privacy settings), so the guard rejects browser-driven cross-origin requests like a malicious local webpage `fetch`-ing `http://127.0.0.1:7878/tool/save_baseline` to mutate `.codemap/index.db`. `Host` mismatch on a loopback bind blocks DNS rebinding (an attacker resolving `evil.com` to `127.0.0.1` post-load). Non-browser clients (curl, fetch from Node, MCP hosts, CI scripts) typically omit these headers and pass through. Implementation: `src/cli/cmd-serve.ts` (CLI shell) + `src/application/http-server.ts` (transport). See [`architecture.md` § HTTP wiring](./architecture.md#cli-usage).
568+
Long-running HTTP server exposing the same tool taxonomy as `codemap mcp` over `POST /tool/{name}` for non-MCP consumers (CI scripts, simple `curl`, IDE plugins that don't speak MCP). Default bind **`127.0.0.1:7878`** (loopback only — refuse `0.0.0.0` unless explicitly opted in via `--host 0.0.0.0`; any **`127.0.0.0/8`** address counts as loopback for the token rule); `--token <secret>` is optional on loopback binds and **required** on non-loopback binds — when set, every request needs `Authorization: Bearer <secret>`. HTTP returns each tool's native JSON payload directly (NOT MCP's `{content: [...]}` wrapper); `query` / `query_recipe` match `codemap query --json` row arrays unless `summary` / `group_by` reshape the envelope, or `baseline` returns a diff envelope (incompatible with non-`json` `format` / `group_by`; save/list/drop remain separate tools); parity twins (`query batch`, `trace`, `explore`, `node`, `file`, `schema`, `symbols`, `context`, `ingest-coverage`, `ingest-churn`) always emit JSON on CLI without `--json`; other tools match their CLI `--json` payloads when that flag is set; `format: "sarif"` payloads ship as `application/sarif+json`, `format: "annotations"` / `"mermaid"` / `"diff"` / `"badge"` (markdown) as `text/plain; charset=utf-8`, `format: "diff-json"` / `"codeclimate"` / `"badge"` + `badge_style: "json"` as `application/json; charset=utf-8`. Routes: `POST /tool/{name}` (every MCP tool), `GET /resources/{encoded-uri}` (resource handler for `codemap://recipes`, `codemap://recipes/{id}`, `codemap://schema`, `codemap://skill`, `codemap://rule`, `codemap://mcp-instructions`, `codemap://files/{path}`, and `codemap://symbols/{name}`), `GET /health` (auth-exempt liveness probe — does not start the watcher), `GET /tools` / `GET /resources` (catalogs). With `--watch`, chokidar is refcount-gated per request and stops 5s after the last client (`HTTP_WATCH_RELEASE_GRACE_MS`) — distinct from MCP idle shutdown; the HTTP process keeps listening. Pure transport — same `tool-handlers.ts` / `resource-handlers.ts` MCP uses; no engine duplication. Errors → `{"error": "..."}` with HTTP status 400 / 401 / 403 / 404 / 500. SIGINT / SIGTERM → graceful drain. Every response carries `X-Codemap-Version: <semver>`. **CSRF + DNS-rebinding guard:** every request (including auth-exempt `/health`) is evaluated against `Sec-Fetch-Site` / `Origin` / `Host` when present — modern browsers send `Sec-Fetch-Site` and `Origin` on cross-origin fetches (header presence varies by request type, browser, and privacy settings), so the guard rejects browser-driven cross-origin requests like a malicious local webpage `fetch`-ing `http://127.0.0.1:7878/tool/save_baseline` to mutate `.codemap/index.db`. `Host` mismatch on a loopback bind blocks DNS rebinding (an attacker resolving `evil.com` to `127.0.0.1` post-load). Non-browser clients (curl, fetch from Node, MCP hosts, CI scripts) typically omit these headers and pass through. Implementation: `src/cli/cmd-serve.ts` (CLI shell) + `src/application/http-server.ts` (transport). See [`architecture.md` § HTTP wiring](./architecture.md#cli-usage).
569569

570570
### Code Climate format (`codeclimate`)
571571

docs/plans/security-hardening-wave1.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ codemap serve --host 0.0.0.0
3131
→ parseServeRest: require --token when !isLoopbackHost(host)
3232
3333
codemap validate <paths>
34-
→ computeValidateRows → rejectValidatePath
35-
→ pathEscapesProjectRoot | pathTraversesSymlinkOutsideRoot → status rejected
34+
→ computeValidateRows → rejectUnsafeProjectRelativePath / readUtf8WithinProjectRoot
35+
→ pathEscapesProjectRoot | pathTraversesSymlinkOutsideRoot | realpath → status rejected
3636
```
3737

3838
---
@@ -46,7 +46,7 @@ codemap validate <paths>
4646
| 2.1 | `isLoopbackHost` + non-loopback `--token` required | **done** | `bun test src/cli/cmd-serve.test.ts` |
4747
| 2.2 | Serve tests updated | **done** | same |
4848
| 3.1 | `pathTraversesSymlinkOutsideRoot`, `resolvePathWithinRoot` | **done** | `bun test src/application/path-containment.test.ts` |
49-
| 3.2 | `validate` `rejected` + `rejectValidatePath` | **done** | `bun test src/cli/cmd-validate.test.ts` |
49+
| 3.2 | `validate` `rejected` + safe read containment | **done** | `bun test src/cli/cmd-validate.test.ts` |
5050
| 3.3 | Test `../../../etc/passwd``rejected` | **done** | same |
5151
| 1.x | `docs/architecture.md` lift | **done** | `bun run format:check` |
5252
| 1.s | Commit + PR + CI | **pending** | `bun run check` (committed; PR + CI open) |

src/application/mcp-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ function registerValidateTool(server: McpServer): void {
267267
"validate",
268268
withToolAnnotations("validate", {
269269
description:
270-
"Compare on-disk SHA-256 of indexed files to the indexed `files.content_hash` column. Returns only out-of-sync rows with status `stale` / `missing` / `unindexed` / `rejected` (fresh paths omitted; `rejected` includes optional `reason` when a path escapes the project root or resolves outside via symlink). Empty `paths` validates every indexed file. Useful for 'codemap doctor' agents that diagnose a stale index before issuing structural queries.",
270+
"Compare on-disk SHA-256 of indexed files to the indexed `files.content_hash` column. Returns only out-of-sync rows with status `stale` / `missing` / `unindexed` / `rejected` (fresh paths omitted; `rejected` includes optional `reason`: path escapes project root | path escapes via symlink | path resolves outside project root). Output `path` keys are project-relative POSIX paths. Empty `paths` validates every indexed file. Useful for 'codemap doctor' agents that diagnose a stale index before issuing structural queries.",
271271
inputSchema: validateArgsSchema,
272272
}),
273273
(args) => wrapToolResult(handleValidate(args)),

src/application/path-containment.test.ts

Lines changed: 133 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ import { mkdirSync, mkdtempSync, symlinkSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55

6+
import { canCreateSymlinks } from "../test/symlink-capable";
67
import {
8+
pathRealpathEscapesProjectRoot,
79
pathTraversesSymlinkOutsideRoot,
810
projectRelativePathFromResolved,
11+
readUtf8WithinProjectRoot,
12+
rejectUnsafeProjectRelativePath,
913
resolvePathWithinRoot,
1014
} from "./path-containment";
1115

@@ -24,46 +28,131 @@ describe("projectRelativePathFromResolved", () => {
2428
});
2529
});
2630

31+
const symlinkCapable = canCreateSymlinks();
32+
2733
describe("pathTraversesSymlinkOutsideRoot", () => {
28-
it("returns false for symlinks that stay inside the project root", () => {
29-
const root = mkdtempSync(join(tmpdir(), "codemap-symlink-in-"));
30-
writeFileSync(join(root, "real.ts"), "export const x = 1;\n");
31-
symlinkSync(join(root, "real.ts"), join(root, "link.ts"));
32-
expect(pathTraversesSymlinkOutsideRoot(root, join(root, "link.ts"))).toBe(
33-
false,
34-
);
34+
it.skipIf(!symlinkCapable)(
35+
"returns false for symlinks that stay inside the project root",
36+
() => {
37+
const root = mkdtempSync(join(tmpdir(), "codemap-symlink-in-"));
38+
writeFileSync(join(root, "real.ts"), "export const x = 1;\n");
39+
symlinkSync(join(root, "real.ts"), join(root, "link.ts"));
40+
expect(pathTraversesSymlinkOutsideRoot(root, join(root, "link.ts"))).toBe(
41+
false,
42+
);
43+
},
44+
);
45+
46+
it.skipIf(!symlinkCapable)(
47+
"returns true when a path component symlinks outside the project root",
48+
() => {
49+
const base = mkdtempSync(join(tmpdir(), "codemap-symlink-out-"));
50+
const root = join(base, "proj");
51+
const outside = join(base, "outside");
52+
mkdirSync(root, { recursive: true });
53+
mkdirSync(outside, { recursive: true });
54+
writeFileSync(join(outside, "secret.ts"), "export const s = 1;\n");
55+
symlinkSync(join(outside, "secret.ts"), join(root, "escape.ts"));
56+
expect(
57+
pathTraversesSymlinkOutsideRoot(root, join(root, "escape.ts")),
58+
).toBe(true);
59+
},
60+
);
61+
62+
it.skipIf(!symlinkCapable)(
63+
"returns true when an intermediate directory symlinks outside the root",
64+
() => {
65+
const base = mkdtempSync(join(tmpdir(), "codemap-symlink-dir-"));
66+
const root = join(base, "proj");
67+
const outside = join(base, "outside");
68+
mkdirSync(root, { recursive: true });
69+
mkdirSync(join(outside, "nested"), { recursive: true });
70+
writeFileSync(
71+
join(outside, "nested", "secret.ts"),
72+
"export const s = 1;\n",
73+
);
74+
symlinkSync(outside, join(root, "linked-dir"));
75+
expect(
76+
pathTraversesSymlinkOutsideRoot(
77+
root,
78+
join(root, "linked-dir", "nested", "secret.ts"),
79+
),
80+
).toBe(true);
81+
},
82+
);
83+
84+
it.skipIf(!symlinkCapable)(
85+
"returns true for a broken symlink in the path chain",
86+
() => {
87+
const root = mkdtempSync(join(tmpdir(), "codemap-symlink-broken-"));
88+
symlinkSync(join(root, "missing-target.ts"), join(root, "broken.ts"));
89+
expect(
90+
pathTraversesSymlinkOutsideRoot(root, join(root, "broken.ts")),
91+
).toBe(true);
92+
},
93+
);
94+
});
95+
96+
describe("pathRealpathEscapesProjectRoot", () => {
97+
it.skipIf(!symlinkCapable)(
98+
"returns true when realpath follows a symlink outside the project root",
99+
() => {
100+
const base = mkdtempSync(join(tmpdir(), "codemap-realpath-out-"));
101+
const root = join(base, "proj");
102+
const outside = join(base, "outside");
103+
mkdirSync(root, { recursive: true });
104+
mkdirSync(outside, { recursive: true });
105+
writeFileSync(join(outside, "secret.ts"), "export const s = 1;\n");
106+
symlinkSync(join(outside, "secret.ts"), join(root, "escape.ts"));
107+
expect(
108+
pathRealpathEscapesProjectRoot(root, join(root, "escape.ts")),
109+
).toBe(true);
110+
expect(rejectUnsafeProjectRelativePath(root, "escape.ts")).toBe(
111+
"path escapes via symlink",
112+
);
113+
},
114+
);
115+
});
116+
117+
describe("readUtf8WithinProjectRoot", () => {
118+
it("reads safe files via realpath", () => {
119+
const root = mkdtempSync(join(tmpdir(), "codemap-read-safe-"));
120+
writeFileSync(join(root, "a.ts"), "export const a = 1;\n");
121+
const result = readUtf8WithinProjectRoot(root, "a.ts");
122+
expect(result).toEqual({ ok: true, content: "export const a = 1;\n" });
123+
});
124+
125+
it("returns missing for paths that do not exist", () => {
126+
const root = mkdtempSync(join(tmpdir(), "codemap-read-missing-"));
127+
expect(readUtf8WithinProjectRoot(root, "gone.ts")).toEqual({
128+
ok: false,
129+
status: "missing",
130+
});
35131
});
36132

37-
it("returns true when a path component symlinks outside the project root", () => {
38-
const base = mkdtempSync(join(tmpdir(), "codemap-symlink-out-"));
133+
it.skipIf(!symlinkCapable)("rejects broken symlinks before read", () => {
134+
const root = mkdtempSync(join(tmpdir(), "codemap-read-broken-"));
135+
symlinkSync(join(root, "missing.ts"), join(root, "broken.ts"));
136+
expect(readUtf8WithinProjectRoot(root, "broken.ts")).toEqual({
137+
ok: false,
138+
status: "rejected",
139+
reason: "path escapes via symlink",
140+
});
141+
});
142+
143+
it.skipIf(!symlinkCapable)("rejects symlink escapes before read", () => {
144+
const base = mkdtempSync(join(tmpdir(), "codemap-read-escape-"));
39145
const root = join(base, "proj");
40146
const outside = join(base, "outside");
41147
mkdirSync(root, { recursive: true });
42148
mkdirSync(outside, { recursive: true });
43149
writeFileSync(join(outside, "secret.ts"), "export const s = 1;\n");
44150
symlinkSync(join(outside, "secret.ts"), join(root, "escape.ts"));
45-
expect(pathTraversesSymlinkOutsideRoot(root, join(root, "escape.ts"))).toBe(
46-
true,
47-
);
48-
});
49-
50-
it("returns true when an intermediate directory symlinks outside the root", () => {
51-
const base = mkdtempSync(join(tmpdir(), "codemap-symlink-dir-"));
52-
const root = join(base, "proj");
53-
const outside = join(base, "outside");
54-
mkdirSync(root, { recursive: true });
55-
mkdirSync(join(outside, "nested"), { recursive: true });
56-
writeFileSync(
57-
join(outside, "nested", "secret.ts"),
58-
"export const s = 1;\n",
59-
);
60-
symlinkSync(outside, join(root, "linked-dir"));
61-
expect(
62-
pathTraversesSymlinkOutsideRoot(
63-
root,
64-
join(root, "linked-dir", "nested", "secret.ts"),
65-
),
66-
).toBe(true);
151+
expect(readUtf8WithinProjectRoot(root, "escape.ts")).toEqual({
152+
ok: false,
153+
status: "rejected",
154+
reason: "path escapes via symlink",
155+
});
67156
});
68157
});
69158

@@ -81,14 +170,17 @@ describe("resolvePathWithinRoot", () => {
81170
expect(resolvePathWithinRoot(root, "../../../etc/passwd")).toBeNull();
82171
});
83172

84-
it("returns null when a symlink target resolves outside the root", () => {
85-
const base = mkdtempSync(join(tmpdir(), "codemap-resolve-symlink-"));
86-
const root = join(base, "proj");
87-
const outside = join(base, "outside");
88-
mkdirSync(root, { recursive: true });
89-
mkdirSync(outside, { recursive: true });
90-
writeFileSync(join(outside, "secret.ts"), "export const s = 1;\n");
91-
symlinkSync(join(outside, "secret.ts"), join(root, "escape.ts"));
92-
expect(resolvePathWithinRoot(root, "escape.ts")).toBeNull();
93-
});
173+
it.skipIf(!symlinkCapable)(
174+
"returns null when a symlink target resolves outside the root",
175+
() => {
176+
const base = mkdtempSync(join(tmpdir(), "codemap-resolve-symlink-"));
177+
const root = join(base, "proj");
178+
const outside = join(base, "outside");
179+
mkdirSync(root, { recursive: true });
180+
mkdirSync(outside, { recursive: true });
181+
writeFileSync(join(outside, "secret.ts"), "export const s = 1;\n");
182+
symlinkSync(join(outside, "secret.ts"), join(root, "escape.ts"));
183+
expect(resolvePathWithinRoot(root, "escape.ts")).toBeNull();
184+
},
185+
);
94186
});

0 commit comments

Comments
 (0)