Repository navigation
Commit 9d5b6a6
committed
fix(mcode-trajectory-studio): contain file reads, redact stored credential forms, gate the panel on a capability
Addresses the review on MiniMax-AI#56. Every finding was reproduced before the change and
re-checked after it, and the new suites are written so that "refuse everything"
cannot pass them.
**Symlink root escapes** (`tasks.mjs`, `jsonl.mjs`). Every file read now goes through
`containedRealPath` / `openContainedRead` in `fsutil.mjs` and has to canonicalize
inside the data directory, with a non-symlink final component opened under
`O_NOFOLLOW` so the containment decision and the read describe one inode. Three
separate defects were reproduced: a symlinked task directory, a symlinked task root,
and a symlinked session directory. The session level of the JSONL walk had no
`isDirectory()` guard at all, and the `stat()` before `isSymbolicLink()` was dead
code, because `stat` resolves the link it was asked to detect.
**Redaction missed the encodings credentials take on disk.** The key/value rule
required the key and its separator to be adjacent, so `{"api_key":"…"}` never
matched, and the Authorization rule ran after the key/value rule, which consumed
`Authorization` and left the token of `Authorization: Bearer …` in the clear. The
rules are now ordered most-specific-first and shape-preserving, task descriptions and
commands are redacted at the data source, a session title has only its credential
substrings replaced, and every outbound payload is swept once more at the boundary so
a field added later cannot escape. One defect the browser run caught and no unit test
would have: key-name redaction matched `sessionId`, so `/api/sessions` returned
`"[redacted]"` and every following request 404'd.
**The panel's fences were CSRF defences, not authorization.** Any local process could
send the fixed header and read every session's tool arguments. A per-process 256-bit
capability is now minted on each start, carried in the URL fragment (so it is never
sent to the server, logged, or leaked through `Referer`) and required, constant-time,
on every API route. mcode spawns one MCP server per session, so two sessions get two
panels with two capabilities and neither URL opens the other's panel.
**`trajectory_studio` declared `readOnlyHint: true` while opening a listener.** The
annotation is corrected, and the port file is gone rather than hardened: once the
capability is per process, reusing a previous process's port is semantically wrong
(the old token is gone and the new one was never sent to the old page), and removing
the write removes the symlink hazard. The Plugin now creates no files.
**The Node range was overstated.** Measured across nine releases: `node:sqlite` needs
22.13.0, and FTS5 is not monotonic in the Node version — absent in 22.13.0 through
22.18.x and throughout 23.x, present from 22.19.0 and 24.0.0. The floor is enforced
with a readable message instead of a module-resolution crash, the verified range
mirrors mcode's own `engines` (`>=22.19 <23 || >=24 <27`), FTS5 is probed rather than
inferred, and search degrades with a warning where it is absent. Found while
measuring: SQLite needs a writable *directory* even for a read-only open, because WAL
needs its `-shm` file, now documented as a limitation.
**Render layer, unchanged but now guarded.** The client already built every node with
`createElement` and wrote only `textContent`; a mutation-checked scan now fails the
suite if that ever stops being true, and the panel binds `127.0.0.1` as a
non-overridable constant and refuses a non-loopback peer address.
**New evidence.** `test/protocol.test.mjs` spawns the real MCP server over stdio the
way mcode does and drives it as a client: version negotiation, the seven declared
tools and their annotations, a call against a fixture projection, full detail redacted
on the wire while the payload survives, an unknown tool and an unknown method, a
notification left unanswered, `--doctor`, and the lifecycle — closing stdin has to end
the process *with the panel running*, because a listener left behind leaks a port on
every session. Every wait is bounded, so a server that stops answering fails the suite
instead of hanging it. `tools/fixture.mjs` now owns the runtime-shaped projection that
the domain tests, the protocol suite and the E2E harness share, and
`tools/compat-matrix.mjs` asserts the compatibility table rather than printing it
(fail === 0; skip count matching whether this runtime's SQLite has FTS5; every skip
being the FTS5 search test). It is mutation-checked: an injected failure and an
injected unrelated skip both make it exit 1.
**Cross-platform.** This suite had never run off Linux. Its first run on Windows and
macOS found three platform assumptions, all of them in the tests rather than the
Plugin: a temporary path compared before canonicalization (`/var` on macOS, a short
`RUNNER~1` path on Windows), a POSIX-only assertion that a resolved data directory
comes back verbatim, and a temporary directory removed while SQLite still held the
file open (`EBUSY` on Windows, harmless on POSIX — it cascaded through 24 tests).
All three are fixed; both platforms now report 115 tests, 0 failures.
No workflow is added to this repository: what runs on the project's runners is the
maintainers' call. The job definitions are written out in the pull request instead.
Tests: 115 in the Plugin suite, 472 repo-wide, 0 failures. Verified on Node 22.12.0
(refused with a reason), 22.13.0, 22.15.0, 22.19.0, 22.21.1, 22.23.2, 23.4.0, 23.11.0,
24.0.0, 24.16.0, 24.19.0 and 24.20.0, with the outcome matching the documented range
on each. Version 0.1.1.1 parent 8b7cd94 commit 9d5b6a6
32 files changed
Lines changed: 2829 additions & 330 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 | | - | |
4 | | - | |
| 3 | + | |
| 4 | + | |
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
322 | 322 | | |
323 | 323 | | |
324 | 324 | | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
329 | | - | |
330 | | - | |
331 | | - | |
332 | | - | |
333 | | - | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
334 | 378 | | |
335 | 379 | | |
336 | 380 | | |
| |||
0 commit comments