Repository navigation
Commit 4dd8dd4
committed
fix(mcode-trajectory-studio): close the confidentiality, bounding and JSONL-reporting defects
One commit for the work on this branch since the last reviewed head. Nothing outside
`plugins/weekbin/mcode-trajectory-studio/` is touched.
Redaction, tuned to the form credentials actually take on disk
- The redactor could not see that form. The runtime stores tool results as JSON *text*:
in a real projection, 109,462 of 118,109 message rows carry `tool_call_result_data` as
a string, so a credential arrives escaped — `{\"api_key\":\"…\"}` — and a key/value rule
that requires the key and its separator to be adjacent never fires. A string value that
parses as JSON is now walked as structured data (rewritten only when a redaction
actually fired, so JSON tool results are never reformatted to protect nothing), and the
key/value rule tolerates and re-emits any backslash run on either delimiter. Measured
through the shipped MCP path: the escaped pairs whose key names a credential go from
none redacted to 54 pairs, 0 surviving.
- A permissive value pattern let the *outer* pair of an escaped document match first, so
the scanner stepped past the sensitive pair nested inside it. There is a regression test
for that shape.
- Rules were not idempotent: the unquoted value class stops at `]`, so `alreadyRedacted`
never saw a complete marker and every sweep added a bracket; and a marker a small
`maxLength` had cut short was re-redacted on the next pass, shifting the omitted count.
Both fixed at the cause, and idempotence is asserted as a property over the corpus ×
four option sets rather than by inspection.
- Structured keys are judged by word-boundary containment instead of an exact-name set, so
`clientSecret`, `refreshToken`, `accessToken`, `authToken`, `privateKey`, `apiSecret` and
`xApiKey` are covered, while counters (`inputTokens`, `total_tokens`) and the addressing
identifier (`sessionId`) are not — treating a session id as a secret returns
`"[redacted]"` from `/api/sessions` and breaks every following request. A task's
description and command are redacted at the data source, so no consumer can forget.
- Added coverage for `github_pat_`, `npm_`, `hf_`, Google `AIza`/`ya29.`, Azure
`AccountKey=` and an unlabelled JWT, and a connection string whose password itself
contains `@` no longer stops at the first `@` and leaves the tail in the clear. Over-
redaction is pinned from the other side: `npm_config_registry` and `HF_HOME` survive
intact and a 13-digit epoch is not read as a phone number.
- E-mail addresses and phone numbers are masked on the MCP egress only, because that
output reaches a model context while the panel is the reader's own screen. The home
directory, the data directory and operator-configured roots fold to `~` wherever they
appear — including the doubled-backslash form a Windows path takes inside a JSON column
— and another account's home keeps its shape and loses the name.
- `/api/overview` returned `stats.workspaceDir` raw beside a `session.workspaceDir` that
was already folded; both are folded now, with an HTTP regression.
Bounding what can be reached
- Per-string limits were not a response limit: 1000 full records serialised to ~20 MB. An
aggregate byte budget plus depth and entry ceilings were added; the page is trimmed in
order and reports `truncated`/`omitted` so the client pages on `nextOffset`. MCP emits
the payload twice (text and `structuredContent`), so its record-list budget is half.
- The SQLite read had no per-row ceiling, so one multi-megabyte row was materialised and
parsed whole. The size test now runs inside SQL and an oversized row is returned as
`oversized` with its byte count rather than silently dropped.
- The `git rev-parse` probe inherited the whole `process.env` on a directory that came out
of session data. It now receives an exact allowlist plus fixed `-c core.fsmonitor=false
-c credential.helper=`. The user's `~/.gitconfig` is deliberately kept (`safe.directory`
lives there); an `alias.rev-parse` in a repository's own config cannot shadow the
builtin, which was measured.
- Diagnostics were unbounded — one warning per failed read, echoed in full on every
`trajectory_list`. The list is capped and reports its dropped count.
The JSONL line cap, and its reporting
- The cap is enforced per chunk rather than after appending, so a single unterminated line
no longer grows the buffer to the size of the file. Measured inside an instrumented copy
of the module: the pending buffer peaks at 2.06 MiB (the cap plus one chunk) for a 10,
100 and 400 MiB stream.
- An oversized line whose newline arrived in the same chunk as the byte that crossed the
cap was discarded *without being counted*: only the pending-buffer path incremented
`droppedOversized`, so which path a line took — counted or silent — depended on where the
chunk boundary fell. Both paths count now, they cannot double count (the discarding
branch consumes the overflowing line's newline and returns before the per-line test),
and a test asserts the two deliveries agree.
Containment, git's environment, and the panel
- On Linux the opened descriptor is re-verified through `/proc/self/fd`, so an intermediate
directory swapped between canonicalization and open is refused; where `/proc` is absent
that residual race is the documented boundary instead of an unstated one.
- The panel opens in `summary` detail. The markup shipped the toggle `checked` while the
state defaulted to `full`, so the panel fetched content the control said it was not
fetching; the control is derived from the state now so the two cannot drift apart.
Docs
- README (both languages) carries the confidentiality contract and its known limits;
DESIGN.md gains the threat model and the accepted boundaries, and the validator section
is renumbered 6.6 → 6.7 because the inserted section took 6.6.
- An unused export that returned an unredacted, unbounded object was removed rather than
left as a channel that would be unredacted the day someone wired it up.
Tests: 166 plugin (was 138 at the reviewed head), 523 repo-wide, 0 failures.
tools/compat-matrix.mjs asserts 166 pass / 0 skip on this runtime.1 parent 9d5b6a6 commit 4dd8dd4
29 files changed
Lines changed: 1982 additions & 206 deletions
File tree
- plugins/weekbin/mcode-trajectory-studio
- .claude-plugin
- server
- skills/mcode-trajectory-studio
- test
- tools
- web
- js
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
187 | 187 | | |
188 | 188 | | |
189 | 189 | | |
190 | | - | |
| 190 | + | |
191 | 191 | | |
192 | 192 | | |
193 | 193 | | |
| |||
270 | 270 | | |
271 | 271 | | |
272 | 272 | | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
273 | 278 | | |
274 | 279 | | |
275 | 280 | | |
| |||
283 | 288 | | |
284 | 289 | | |
285 | 290 | | |
286 | | - | |
| 291 | + | |
287 | 292 | | |
288 | 293 | | |
289 | | - | |
| 294 | + | |
290 | 295 | | |
291 | 296 | | |
292 | 297 | | |
| |||
345 | 350 | | |
346 | 351 | | |
347 | 352 | | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
348 | 356 | | |
349 | 357 | | |
350 | 358 | | |
351 | | - | |
| 359 | + | |
352 | 360 | | |
353 | 361 | | |
354 | 362 | | |
355 | 363 | | |
356 | | - | |
357 | | - | |
358 | | - | |
359 | | - | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
360 | 387 | | |
361 | | - | |
362 | | - | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
363 | 436 | | |
364 | 437 | | |
365 | 438 | | |
| |||
373 | 446 | | |
374 | 447 | | |
375 | 448 | | |
376 | | - | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
377 | 454 | | |
378 | 455 | | |
379 | | - | |
| 456 | + | |
380 | 457 | | |
381 | 458 | | |
382 | 459 | | |
| |||
0 commit comments