Conversation
Port the legacy HTTP endpoint (stubbed in 56ea465 / v0.2.578) to the new `std.Io.net` surface: bind 127.0.0.1 with `IpAddress.parse`/`listen`, accept in a loop, and hand each stream to a detached thread. The routes and JSON response shapes match the pre-0.16 implementation so existing clients don't need changes. Refs #307, #285 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
For `codedb mcp`, stdout is reserved for JSON-RPC messages. The root-policy failure path wrote `✗ refusing to index temporary root: …` (and the normal `✓ indexed` startup line) to stdout, which hosts reject with `invalid character 'â' looking for beginning of value` on the leading UTF-8 byte of the status glyph. Switch `out.file` to stderr once `cmd == "mcp"` is resolved, so every `out.p` call on that path goes to stderr while stdout stays clean for protocol messages. Closes #304 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Read `CODEDB_PORT` from the environment; fall back to 7719 on absence or parse failure. Unblocks running multiple instances on one host, reverse-proxy setups, and integration tests that need an ephemeral port. Refs #308 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`Explorer.findSymbol` now looks up the name in `self.symbol_index` and builds results from the cached locations. The full outline scan is kept as a fallback for safety. For the index to be authoritative, `rebuildSymbolIndexFor` no longer skips `.import` / `.comment_block` kinds — those were being missed by the O(1) path and forced callers into the slow scan. Indexing every kind makes results match the scan-based path exactly. Refs #309 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Benchmark Regression ReportThreshold: 10.00%
|
Drop the hardcoded 7719 fallback. If CODEDB_PORT is unset, `codedb serve` exits with a clear message explaining how to enable it (suggested 47719, since 7719 and 8080 tend to collide with other local processes). If set but not parseable as u16, exit with an error. Rationale: the HTTP server opens a network port; having it bind on a predictable default when someone runs `codedb serve` accidentally is worth avoiding. Treating the env var as the on/off switch keeps the surface area minimal and makes the enabled case explicit in shell history / process listings. Refs #308 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Benchmark Regression ReportThreshold: 10.00%
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9d773c9ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var it = std.mem.splitScalar(u8, path, '/'); | ||
| while (it.next()) |component| { | ||
| if (std.mem.eql(u8, component, "..")) return false; | ||
| } |
There was a problem hiding this comment.
server.isPathSafe only rejects absolute / paths and .. segments split on '/', so inputs like ..\\..\\secret.txt are accepted. On Windows, \ is a real path separator, so this bypass can reach files outside the intended tree through /file/read and /edit. src/mcp.zig:isPathSafe already rejects backslashes; the HTTP server should enforce the same rule to avoid traversal gaps.
Useful? React with 👍 / 👎.
| if (self.symbol_index.get(name)) |locs| { | ||
| for (locs.items) |loc| { | ||
| var detail: ?[]const u8 = null; | ||
| if (self.outlines.getPtr(loc.path)) |outline| { |
There was a problem hiding this comment.
This new early return assumes symbol_index is authoritative, but that is not always true after fast snapshot restore: restored files are inserted into outlines without rebuilding symbol_index, and later watcher/edit updates repopulate the map only for touched files. In that mixed state, symbols present in untouched files are silently omitted because the outline scan fallback never runs once get(name) is non-null. Keep scanning for missing hits or rebuild the full symbol index after fast restore.
Useful? React with 👍 / 👎.
| respondJson(&conn, "403 Forbidden", "{\"error\":\"path traversal not allowed\"}"); | ||
| return; | ||
| } | ||
| const content = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(10 * 1024 * 1024)) catch |err| switch (err) { |
There was a problem hiding this comment.
/file/read currently uses std.Io.Dir.cwd() to open files, but codedb <root> serve indexes paths relative to the provided root. If the process is launched from another directory, valid indexed paths are resolved against the wrong base (causing false 404s or reading the wrong file). The read handler should use the same root directory context used by indexing/watching.
Useful? React with 👍 / 👎.
Running `codedb serve` is itself the opt-in — codedb has no always-on daemon, so gating the listener behind an additional CODEDB_PORT requirement was belt-and-suspenders with no threat to block. Restore the previous UX: `codedb serve` starts listening on a default port, CODEDB_PORT stays as an optional override for collisions. Default is now 6767 (picked off the beaten path — 7719 and 8080 collided with other local tooling). Refs #308 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`isPathSafe` only rejected absolute `/` paths and `..` segments split on `/`, so inputs like `..\\..\\secret.txt` passed through — on platforms where `\\` is a real separator this could reach files outside the indexed tree through `/file/read` and `/edit`. Null bytes likewise could truncate paths in downstream syscalls. Mirror `mcp.isPathSafe`: reject null bytes and backslashes up front before the `/`-split loop. Addresses Codex P1 on #310. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The HTTP handler was opening files via `std.Io.Dir.cwd()`, but `codedb <root> serve` indexes paths relative to the provided root. Launched from any other directory, valid indexed paths hit the wrong base and returned false 404s (or worse — read the wrong file). Open via `explorer.root_dir` instead. Respond 500 with a clear error if the root was never configured (shouldn't happen on the normal serve path, but guards against a bare explorer). Addresses Codex P2 on #310. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The earlier change early-returned after the `symbol_index` lookup, but that map can be incomplete after fast-snapshot restore — `outlines` is populated before `rebuildSymbolIndexFor` runs on every file, and later watcher/edit updates only touch files they saw change. Symbols present in untouched files were silently dropped from results once the index had any entry for the name. Keep the O(1) path for the common case, but always fall through into the outline scan and dedupe against a per-call `(path, line_start)` set so the scan fills gaps without duplicating index hits. Addresses Codex P1 on #310. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Benchmark Regression ReportThreshold: 10.00%
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbb8b49b4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if (after) |a| req.after = @intCast(a); | ||
|
|
||
| const result = edit_mod.applyEdit(io, allocator, store, agents, explorer, req) catch |err| { |
There was a problem hiding this comment.
The HTTP /edit handler forwards path to edit_mod.applyEdit, but that helper performs file I/O via std.Io.Dir.cwd() (in src/edit.zig) rather than the indexed root directory. In codedb <root> serve sessions started from another working directory, edits can fail with FileNotFound or modify a same-named file under the process CWD instead of <root>, which can corrupt unrelated files.
Useful? React with 👍 / 👎.
| if (!isPathSafe(path)) { | ||
| respondJson(&conn, "403 Forbidden", "{\"error\":\"path traversal not allowed\"}"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The HTTP handlers only apply traversal checks and do not enforce watcher.isSensitivePath(path), unlike MCP read/edit handlers in src/mcp.zig. This allows localhost clients to read or modify sensitive files (for example .env, *.pem, *.key) through the restored HTTP API even though those paths are treated as protected elsewhere, creating a secret-exposure and credential-tampering gap.
Useful? React with 👍 / 👎.
1 participant
Summary
Trial branch gathering the local-server work that was sitting on the working tree, plus a fix for #304 (MCP stdio stdout contamination) and the three Codex review items.
Commits, each scoped to one change:
c4cc763Restorecodedb serve --porton Zig 0.16 (src/server.zig) — ports the legacy HTTP endpoint (stubbed in 56ea465 / v0.2.578) to the newstd.Io.netsurface. Routes and JSON shapes match the pre-0.16 build. Refs Restorecodedb serve --portHTTP endpoint on Zig 0.16 #307, chore(0.16): MCP + server — mcp.zig, server.zig + Child.run #285.2fbc66cRoute MCP status output to stderr (src/main.zig) — switchesout.fileto stderr oncecmd == "mcp". Closes MCP stdio: failure-path status text is written to stdout and corrupts JSON-RPC #304.aaba92e/14c3160/8b43e89codedb serveport handling (src/main.zig) — lands at: default port 6767,CODEDB_PORTas optional override.codedb serveis its own opt-in; no separate gate. Refs Makecodedb serveport configurable viaCODEDB_PORTenv var #308.c9d773c/fbb8b49O(1)findAllSymbols(src/explore.zig) — index lookup for the common case, with a merging outline scan (deduped on(path, line_start)) so fast-snapshot restore can't drop untouched files.rebuildSymbolIndexForno longer skips.import/.comment_block. RefsfindSymbolshould usesymbol_indexfor O(1) lookup instead of scanning all outlines #309, addresses Codex P1.74ba881Hardenserver.isPathSafe(src/server.zig) — reject null bytes and\\so Windows-style..\\..\\xcan't bypass the/-split..check. Mirrorsmcp.isPathSafe. Addresses Codex P1.6ef7185/file/readresolves against indexed root (src/server.zig) — opens viaexplorer.root_dirinstead ofstd.Io.Dir.cwd(), so launchingcodedb /some/root servefrom elsewhere no longer false-404s or reads the wrong file. Addresses Codex P2.Test plan
zig buildclean on macOS arm64, Zig 0.16.0codedb /private/tmp mcp— stdout: 0 bytes, stderr:✗ refusing to index temporary root: /private/tmpcodedb --mcp </dev/null— stdout: 0 bytes, stderr:info: codedb mcp: root=…codedb serve— binds127.0.0.1:6767;curl :6767/→ JSON 404CODEDB_PORT=12345 codedb serve— binds127.0.0.1:12345curl 'http://127.0.0.1:6767/file/read?path=..\\..\\etc\\passwd'→403 {"error":"path traversal not allowed"}curl 'http://127.0.0.1:6767/file/read?path=src/main.zig'→ 200 (resolved under the indexed root, not cwd)codedb find Explorerstill returns import hits (index + scan both covered)findAllSymbolson a large project to confirm the O(1) win survives the dedup scanNotes
2fbc66c) and the Codex fixes first, and hold the HTTP restore for a separate review. Commits are independently reviewable.zig-pkg/in the working tree was left alone.🤖 Generated with Claude Code