Fix/hunt2 repl

[queil]

No description provided.

[strix-security]

Strix Security Review

Warning

This pull request has 84 commits after the last Strix review (ebbb5aa). Strix has not reviewed these changes.
Automatic review on push is off for this repository. To review the latest changes, tag @strix-security in a comment, or turn on re-review on push.

1 open security finding on this PR:

• 🟢 LOW Destructuring let echo executes command-backed (lazy) bindings, bypassing the REPL's command-echo guard — src/Weir/Repl.fs:2099-2106

Review summary

Reviewed PR #373 (fix/hunt2-repl), which reworks the REPL line editor, history handling, completion quoting, the formatter (Fmt), and the LSP error/lifecycle handling. Most changes are hardening or correctness improvements (path quoting, header-case-insensitive LSP framing, formatter idempotency and BOM preservation, error responses for every request). One finding was identified: the destructuring let echo in src/Weir/Repl.fs now evaluates and renders each binding's value without the command-backing guard that the single-let form applies, so a command-backed (lazy) binding is force-enumerated and its command runs at echo time — e.g. let (a, b) = (1, g) runs g's command immediately, contrary to the documented "command-backed let never runs on echo" behavior.

_{Fixed the findings? re-run the review, or tag @strix-security in a PR comment to run a fresh review.}

_{Updated for ebbb5aa.}

---

Reviewed by Strix
_{Re-run review · Configure security review settings}

[strix-security]

Strix flagged a new security finding below. See the pinned summary comment for the full PR status.

[strix-security]

🟢 Destructuring let echo executes command-backed (lazy) bindings, bypassing the REPL's command-echo guard

Severity: LOW

In the REPL, a destructuring let binding (for example let (a, b) = (1, g) where g is a lazy command-backed value such as let g = sh -c "…") now evaluates and renders each binding's value when echoing the result. Unlike the single-let form, this path never applies the command-backing guard, so rendering the value enumerates it and runs the deferred command immediately at echo time — contradicting the documented behavior that the REPL echo of a command-backed let never runs it.

Suggested change

	for n, sch in schemes do
	Console.WriteLine $"{n} : {formatEchoTy sch.Ty}"
	match bindings |> List.tryFind (fun (bn, _) -> bn = n) with
	| Some(_, bv) when bv <> Eval.VUnit ->
	let rendered, hint = Eval.echoValue cap (Eval.echoPrep bv)
	Console.WriteLine
	$"{n} : {formatEchoTy sch.Ty} = {Eval.sanitizeTtyData rendered}{Eval.echoTail hint}"
	| _ -> Console.WriteLine $"{n} : {formatEchoTy sch.Ty}"
	for n, sch in schemes do
	match bindings |> List.tryFind (fun (bn, _) -> bn = n) with
	| Some(_, bv) when bv <> Eval.VUnit ->
	if letCommandBacked te bv then
	noteCommandSeq n te bv
	echoMeta (commandBackedEcho n sch.Ty)
	else
	let rendered, hint = Eval.echoValue cap (Eval.echoPrep bv)
	Console.WriteLine
	$"{n} : {formatEchoTy sch.Ty} = {Eval.sanitizeTtyData rendered}{Eval.echoTail hint}"
	| _ -> Console.WriteLine $"{n} : {formatEchoTy sch.Ty}"

Prompt to fix with AI

This is a security vulnerability found during a code review.

Vulnerability: Destructuring `let` echo executes command-backed (lazy) bindings, bypassing the REPL's command-echo guard
Severity: LOW

In the REPL, a destructuring `let` binding (for example `let (a, b) = (1, g)` where `g` is a lazy command-backed value such as `let g = sh -c "…"`) now evaluates and renders each binding's value when echoing the result. Unlike the single-`let` form, this path never applies the command-backing guard, so rendering the value enumerates it and runs the deferred command immediately at echo time — contradicting the documented behavior that the REPL echo of a command-backed `let` never runs it.

Location: src/Weir/Repl.fs:2099-2106
Context: Destructuring let echo evaluates bindings without the command-backing guard
```
// Before:
            for n, sch in schemes do
                match bindings |> List.tryFind (fun (bn, _) -> bn = n) with
                | Some(_, bv) when bv <> Eval.VUnit ->
                    let rendered, hint = Eval.echoValue cap (Eval.echoPrep bv)

                    Console.WriteLine
                        $"{n} : {formatEchoTy sch.Ty} = {Eval.sanitizeTtyData rendered}{Eval.echoTail hint}"
                | _ -> Console.WriteLine $"{n} : {formatEchoTy sch.Ty}"
// After:
            for n, sch in schemes do
                match bindings |> List.tryFind (fun (bn, _) -> bn = n) with
                | Some(_, bv) when bv <> Eval.VUnit ->
                    if letCommandBacked te bv then
                        noteCommandSeq n te bv
                        echoMeta (commandBackedEcho n sch.Ty)
                    else
                        let rendered, hint = Eval.echoValue cap (Eval.echoPrep bv)

                        Console.WriteLine
                            $"{n} : {formatEchoTy sch.Ty} = {Eval.sanitizeTtyData rendered}{Eval.echoTail hint}"
                | _ -> Console.WriteLine $"{n} : {formatEchoTy sch.Ty}"
```

How to fix:
Apply the same command-backing guard used by the single-`let` echo path to the destructuring path: for each destructured binding, detect whether its value is a command-backed (unforced) sequence using `letCommandBacked`, and when it is, record the binding via `noteCommandSeq` and print only the type-and-hint metadata (`commandBackedEcho`) instead of enumerating and rendering the value. Pure/forced values should continue to echo normally.

Please fix this vulnerability. If you propose a fix, make it concise and minimal.

_{React 👍 / 👎 to tune Strix for this repo. A repo collaborator (or the PR author) can resolve this thread to dismiss the finding.}