Hardening pass: bounds, containment, and input validation across MCP/CLI/UI#941
Merged
Conversation
The result projection is materialized per row into fixed-width stack arrays (CBM_SZ_32 columns) in execute_return_simple and its siblings, indexed by the parsed RETURN item count — which the parser left unbounded. A RETURN/WITH clause with more than 32 projection items wrote past those arrays. Reject an over-wide projection at parse time, with a fork-isolated regression test driving a 48-column RETURN. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The import path allocated the destination from the plaintext original_size field in artifact.json and passed that value to the decoder truncated through int. A large real database already wraps that cast (a full-kernel index is ~14 GB), and a doctored field could additionally desync the decoder capacity from the actual buffer. Read the decompressed size from the zstd frame's own content-size header, require the metadata field to match it, cap the total, and run the decoder with a size_t capacity equal to the allocation. cbm_zstd_decompress now takes size_t lengths and returns int64_t. Adds a regression test that a mismatched original_size is rejected. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
get_code_snippet resolved a result path and confirmed it stayed under the project root before reading, but the search_code source-attach path built the same kind of path and read it with no such check — so a result whose indexed path resolves outside the root (a `..` segment, or a symlink / junction followed during discovery) could be read back into the response. Extract the existing containment check into a shared cbm_path_within_root() and route both read sinks through it. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
safe_stat rejected POSIX symlinks (lstat + S_ISLNK) but on Windows returned wide_stat directly, which follows junctions / reparse points — so a junction inside an indexed tree could be walked into and files outside the project root pulled into the index. Check FILE_ATTRIBUTE_REPARSE_POINT and skip such entries, mirroring the POSIX symlink skip. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A source path never legitimately contains a newline or carriage return, and on the Windows search filelist those bytes are the per-entry record separator (NUL can't be used there — it breaks PowerShell Get-Content). Skip any indexed path containing one so a crafted path cannot inject an extra file into the scan set. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The graph-UI server binds to loopback only and reflected a localhost Origin for CORS, but did not inspect the Host header — so a request routed to the local port under a foreign name (a rebinding DNS record, a proxy) was served normally, reaching the state-changing endpoints. Parse the Host header and refuse any request whose Host is not a loopback address before dispatch; a request with no Host (local HTTP/1.0 tooling) still proceeds. Adds a live-socket regression test. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
base_branch is spliced into `git diff --name-only "<base>"...HEAD`. The existing check rejected shell metacharacters but not a leading '-', so a value like --output=<path> was passed to git as an option and wrote the diff to an arbitrary file. A real git ref never starts with '-'; reject it alongside the metacharacter check. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The macOS digest path invoked `shasum -a CBM_SZ_256` — a literal string (not a macro; string literals do not expand) naming an invalid algorithm, so the digest always failed on macOS, and the self-update path then treated a could-not-verify result as non-fatal and installed the download regardless. Use the correct algorithm, and install only a positively-verified download: a mismatch, a missing checksum entry, or an unavailable hash tool now all abort. Adds a known-vector digest regression test. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The UI handlers accumulate JSON with `pos += snprintf(buf + pos, size - pos, ...)`. snprintf returns the length it WOULD have written, so on truncation pos ran past the end of the buffer and the next call's `size - pos` wrapped to a huge value and wrote out of bounds — reachable through the directory browser, whose readdir entries can fill the buffer before the trailing parent/roots appends. Route every accumulator through a clamping http_appendf() helper that pins pos at the buffer size on truncation. Adds a forked-child regression test that browses a directory wide enough to fill the buffer. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
index_repository canonicalizes repo_path but imposes no workspace boundary, so a caller-supplied path can resolve anywhere on disk. Add an opt-in CBM_ALLOWED_ROOT: when set, a repo_path that resolves outside it is refused (reusing the same containment check the read sinks use). Unset by default, so the standard index-the-given-path behaviour is unchanged; deployments where repo_path may be influenced by an untrusted caller can set a boundary. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The UI makes no external network calls, but nothing enforced that at the browser. Emit a CSP whose directives name no external host, so a future dependency or injected content cannot load or connect off-origin; connect-src 'self' confines fetch/XHR/WebSocket to the local server. The 'self'/data:/blob:/inline-style/wasm allowances cover the bundled app's own needs (React inline styles, three.js textures/workers). Header emission only, no rendering change expected — confirm the 3D view in a browser before release. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Add CBM_ALLOWED_ROOT to the environment-variable tables in README.md and docs/CONFIGURATION.md: when set, index_repository refuses a repo_path that resolves outside it; unset imposes no restriction. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The policy was only attached to embedded-asset responses, but browsers enforce a CSP from the document response; on subresources it is inert. Attach the same header where index.html is served. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardening and robustness pass across the MCP server, CLI, artifact import, Cypher execution, and the local UI. Each change is small and self-contained, with reproduce-first tests where the behavior is testable.
Changes
search_codefile reads to the project root; skip filelist paths containing control characters; reject an option-likebase_branchindetect_changes; add an optionalCBM_ALLOWED_ROOTboundary forindex_repository.Hoston the local HTTP server; bound the HTTP response accumulators againstsnprintftruncation; send a strict, self-onlyContent-Security-Policywith the served frontend (document + assets).CBM_ALLOWED_ROOTboundary.Testing
Full C suite green locally (incl. ASan); frontend unchanged. New reproduce-first tests cover the cypher bound, artifact size validation, MCP containment/arg checks, the discover reparse skip, the HTTP Host/overflow guards, and the CLI checksum. CSP verified in-browser (3D graph renders, no violations).