chore/fix/refactor: dependency cleanup, auto-fix trust hardening, blame/file-history extraction - #2128
Merged
Merged
Conversation
@langchain/community itself was never imported, but it was transitively providing @langchain/classic and @langchain/textsplitters, which src/lib/langchain/chains/summarize/index.ts and src/lib/parsers/default/index.ts import directly without declaring them. Promote those two to explicit pinned dependencies and drop the unused 22MB @langchain/community package plus its now-orphaned transitive deps (binary-extensions, flat, math-expression-evaluator, uuid). Fixes #1925.
The core exploit (repo-committed .coco.json controlling autoFixTool/ autoFixToolOptions) was already fixed in #2097. This finishes the remaining hardening items from the issue: - Per-adapter option allowlisting: each adapter (codex, claude, gemini) now filters autoFixToolOptions through an explicit allowlist of known tuning flags and drops anything else, warning on the rejected keys. This closes CodexAdapter's `-c key=value` passthrough, which used to forward any unrecognized key straight into codex's config namespace. - TaskList.autoFix() now shows the exact resolved command (binary + args) and requires an explicit confirmation before running it, instead of silently spawning the tool after only printing the finding's title. Adapters expose `buildArgs()`/`binary` and autofix/index.ts exposes a new `prepareAutoFix()` that resolves the command without running it, so the preview reflects the adapter's real logic rather than a reimplementation of it. - A hostileProjectConfig test chains the real trust-boundary filter into the real adapter allowlist (nothing mocked in between) to prove a hostile project config's flags never reach spawn's argv, and a completeness test asserts every schema-visible top-level Config key is explicitly triaged as trusted or untrusted, so a future executing feature can't silently land unprotected the way autoFixTool did. An initial attempt also flipped the project-config trust boundary from a blocklist to an allowlist per the issue's suggestion, but a real attempt broke `noVerify` and would likely break other per-command flags that live outside the schema-visible Config shape — hand-enumerating that whole surface was too risky. The completeness test achieves the same "someone remembered to check" guarantee for the schema-visible surface where the actual #1840 fields lived, without touching the untyped per-command flag surface.
Continues the per-surface extraction of inkInput.ts's router, following the pattern already established for bisect, changelog, rebase, and conflicts. Blame and file-history are the two remaining views the issue calls out as small (7 branches each) — their handlers are interleaved with each other (both are simple linear-list views living side by side at every call site) and, at the jump-top/jump-bottom chords, with changelog's inline handling too. New surfaces/blame/input.ts and surfaces/fileHistory/input.ts each export a slotted handleXInput(state, inputValue, key, context, slot) mirroring handleConflictsInput's non-contiguous-branch pattern. Each slot is a verbatim lift of its original inline block; the existing inkInput.test.ts suite (475 tests) passes unchanged, confirming no behavior drift, plus new dedicated per-slot tests for both handlers. Diff, status, and history remain inline — the largest three surfaces (21/18/17 branches respectively) — as further follow-up work.
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.
What
Three independent, self-contained changes picked up from a GitHub issues review:
@langchain/communitydependency — a 22MB production dependency that's never imported directly.blame/file-historyinput handlers — continues the per-surface extraction ofinkInput.ts's router.Closes #1925
Closes #1840
Part of #1722 —
blame/file-historyare extracted;diff/status/historyremain and are documented as follow-up on the issue (they're far more entangled with each other and other cross-cutting concerns than the surfaces extracted so far, so they weren't a safe verbatim lift under time pressure).How
#1925:
@langchain/communityitself was unused, but transitively supplied@langchain/classicand@langchain/textsplitters, whichsrc/lib/langchain/chains/summarize/index.tsandsrc/lib/parsers/default/index.tsimport directly without declaring. Promoted those two to explicit pinned dependencies (matching the exact versions already validated by CI) and dropped@langchain/communityplus its now-orphaned transitive deps.#1840: The repo-committed-config exploit itself (
autoFixTool/autoFixToolOptionscontrolled by a hostile.coco.json) was already fixed in #2097. This closes the three items the maintainer's own follow-up comment left open:codex/claude/gemini), closingCodexAdapter's unbounded-c key=valuepassthrough into codex's config namespace.TaskList.autoFix()now shows the resolved command (binary + args) and requires explicit confirmation before running, instead of only printing the finding's title.hostileProjectConfig.test.tschains the real trust-boundary filter into the real adapter allowlist (nothing mocked in between) to prove a hostile config's flags never reachspawn's argv, plus a completeness test that fails the build if a new top-levelConfigkey isn't explicitly triaged as trusted/untrusted.noVerifyand would likely break other per-command flags that live outside the schema-visibleConfigshape — reverted that piece in favor of the completeness test, which achieves the same guarantee for the surface where the actual bug lived.)#1722:
changelog/rebase/conflicts/overlay extractions had already landed separately (in #2066, not linked to this issue). This PR extracts the two remaining small surfaces the issue calls out —blameandfile-history(7 branches each) — intosurfaces/blame/input.ts/surfaces/fileHistory/input.ts, each exporting a slottedhandleXInput(state, inputValue, key, context, slot)mirroringhandleConflictsInput's non-contiguous-branch pattern (the two views are interleaved with each other and, at the jump-top/jump-bottom chords, with changelog). Verbatim lifts —inkInput.test.ts's existing 475 cases pass unchanged.Validation
npm testpasses locally (lint + jest + build + packaged-CLI smoke) — 7,023 passed, 14 skipped, 0 failedprepareAutoFix/TaskListconfirm-flow cases, trust-boundary completeness test, per-slotblame/fileHistoryhandler tests)schema.jsonregenerated and diffed — no drift (neither change touches theConfigtype shape)Notes
No breaking changes. The
TaskList.autoFix()confirm-before-run step is a small UX change (auto-fix now requires an explicit confirm even for a well-configured trusted setup) — flagging in case that's worth a mention in release notes.Generated by Claude Code