mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 08:03:37 +00:00
b2d0fd39b64f98c72c969b978f62b92111952fa1
8710
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
b2d0fd39b6 |
chore(ssh): drop the temporary reconnect-replay diagnostic
Served its purpose: it is what turned 'the panes look blank' into replay=false, replayLen=0 on every pane, and then into replay=true with real byte counts once the relay fix landed. The permanent log line keeps the boolean, which is the part worth having. |
||
|
Neil
|
24cdf8ca47 |
fix(ssh): send the pane its screen back on reconnect
A reconnect left every remote terminal blank. Measured on a live relay, not inferred: pty.attach returned no replay for every pane, taking the activation === 'existing' early return in the relay's attach. 'existing' means a source delivery is already open for this client, so it must already be receiving live output and cannot need its screen re-sent. That holds for a duplicate attach. It is false for a reconnect, for a reason neither side can see alone: the client keeps its id across the drop (detachClient refuses to detach the primary, and setWrite revives that same id) so the delivery outlives the dead transport, while the RENDERER has already thrown its terminal away. A reconnect bumps tab.generation, which is the pane's React key, so TerminalPane remounts and the old xterm is disposed with its buffer, and nothing on that path captures it first. Both halves are individually reasonable and together they guarantee a blank pane: the relay reports the client already has the screen, to a client holding a brand-new empty terminal, and nothing paints until new output happens to arrive. Resizing appeared to fix it only because a TUI redraws itself. So the client says which case it is. reattachSshPtySession is by definition painting into a new terminal, so it asks; nobody else does, and the early return keeps working for them. Optional on the wire, so an older relay ignores it and behaves exactly as it does today. Falling through rather than returning the replay inline is deliberate: the path below already drops the pending batched bytes that are also in the buffer, which is what stops the live delivery rendering them twice. Reproduced first as a test against the real dispatcher, source publication and PTY handler (the second attach for one client, which is what a reconnect is) and it fails on the exact symptom before the fix. A second test pins that a caller which does NOT ask still gets nothing, so this cannot become a double-render for the duplicate-attach case the early return exists for. Also updates four provider tests that assert the exact attach params. NOT yet verified in the running app; the log will show replay=true on reconnect. |
||
|
Neil
|
dc6f6bf685 |
test(terminal): pin that panes are PAINTED, not merely bound — and fix a broken commit
Two problems, both mine.
1)
|
||
|
Neil
|
34fc1424f0 |
fix(ssh): repaint background-tab panes revealed after a reconnect
Completes
|
||
|
Neil
|
2fdab478c0 |
fix(ssh): repaint the panes after a reconnect, not just reattach them
Reported: disconnect an SSH host from the Remote Hosts popup, reconnect, and the
terminals come back blank — but resizing a split or toggling the sidebar makes
them render correctly.
That last detail is the diagnosis. The panes were never broken: reattach restores
each pane's buffer but not its painted frame. xterm repaints on a write or a
resize, and a reconnect produces neither for a pane that was already correctly
sized — so nothing paints until a relayout forces it, which is exactly what
resizing or toggling the sidebar does.
The renderer already has refitAndRefreshAllTerminalPanes for this shape ('after
bulk desktop restore, background panes may have correct cols/rows but a stale
xterm renderer until focus forces a repaint'). Its only callers were the mobile
fit-reclaim paths; the SSH reconnect path never used it.
Scheduled from finalizeHydratedTerminalPanes, on both a frame and a 100ms settled
pass — the same pattern the desktop-restore path uses, because rAF alone lands
while panes are still remounting.
Mutation-proved: removing the schedule reddens the new test, which is the
reported symptom.
|
||
|
Neil
|
fa977b6521 |
docs(ssh): correct the design where a live client disproved it
D2, D3 and D4 each stated something about OpenSSH that turned out to be wrong when tested against a real client and sshd rather than read from the source. D3's premise is the notable one: OpenSSH is not type-scoped at all, so it does not avoid the RSA-era false alarm the way the doc claimed. It avoids the situation via order_hostkeyalgs and hard-fails when the situation arises anyway. The conclusion survives — the ordering is still what makes our scoping safe — but for a different reason than the one written down, and a reader would have drawn the wrong lesson. D2 gains the two rules that actually bite: the entry condition to the fallback pass, and HostKeyAlias suppressing the port. D4 records both reversals with their reasoning and the residual risk each one accepts, and the ssh -G spelling trap that made StrictHostKeyChecking dead on arrival. Corrections are kept visible rather than edited out, per the note at the top of the file. |
||
|
Neil
|
924a54167c |
fix(ssh): stop refusing hosts that ssh itself connects to
Two product decisions, both taken deliberately after a review priced their blast radius, and both moving us from stricter-than-ssh to matching it. CERTIFICATE-AUTHORITY HOSTS NO LONGER FAIL. The point of an SSH CA is that the client holds ONE line — very often `@cert-authority *` — instead of per-host entries. That line matches every candidate, so for a Teleport / Vault-SSH / Smallstep / in-house-CA user EVERY target failed, not just CA-signed ones, including on-demand runtime VMs, and StrictHostKeyChecking=no did not help. The documented escape was an environment variable, which an Electron app launched from the Dock or Start Menu never sees. Meanwhile OpenSSH, verified live, treats a CA-covered host presenting a plain key as first contact and connects: ssh2 cannot validate certificates at all, so refusing bought nothing ssh was not already giving up. The residual risk is real and accepted — for a CA-protected host we take a plain key we cannot tie to the CA — and the ca-only outcome is carried through the decision so it stays visible in the log. AN UNREADABLE known_hosts NO LONGER REFUSES EVERYTHING. Any non-ENOENT read error on any configured file rejected every unknown host, with a message blaming the system SSH configuration, which was not what happened. The common trigger is not exotic: a Windows OneDrive Known Folder Move placeholder while offline fails with a cloud-file error, not ENOENT. It was also asymmetric with our own store, which degrades an unreadable file to "nothing trusted" and connects. We now connect as ssh does — it warns and treats the host as unknown — but record NOTHING, so a first contact we could not check never becomes durable trust. That second half is the reason the first is acceptable, so it is pinned end to end with the store actually bound. Which meant splitting verificationSourcesIncomplete back apart. It had been one flag for two claims that now diverge: "a site policy may exist that we cannot read" still refuses, "a file we could not open may contradict this" does not. Merging them was what made the second inherit a strictness only the first justified. Both still lose to evidence we DID read: a mismatch, a revoked key, or an explicit StrictHostKeyChecking still refuse in either state. pnpm lint clean; 1,549 SSH tests pass. |
||
|
Neil
|
1c92304690 |
fix(ssh): only claim the site config was suppressed when ssh -G actually ran
sshGArgsForHost reports which arguments WOULD be used, not what happened. It returns the -F form whenever ~/.ssh/config exists and os.homedir() diverges from the passwd home, so a machine with no usable ssh at all — Windows without OpenSSH, a restricted sandbox, a timed-out probe — was judged by whether it happens to have a ~/.ssh/config, and rejected every unknown host permanently if it did. The same broken machine WITHOUT one stayed fully permissive, which is the tell: the flag is a claim about a config file we could not read, and when ssh never ran there is no such claim to make. Narrow but total where it lands: anything that sets HOME explicitly (wrapper scripts, sudo -E, devcontainers), macOS mobile and network accounts, and the E2E isolation this branch was written for. Found by an availability review. |
||
|
Neil
|
d84645f818 |
fix(ssh): look a HostKeyAlias up the way ssh does — without the port
HostKeyAlias suppresses the port entirely. Verified against OpenSSH 10.2p1 on port 2225 with HostKeyAlias=myalias: an entry keyed `myalias` authenticates, and one keyed `[myalias]:2225` gives "No ED25519 host key is known for myalias". We built [['[alias]:port'], ['alias']] and consulted a form ssh never writes. On its own that was a stale-entry false alarm. The previous commit made it worse: now that the first pass decides as soon as it finds any entry for the host, a leftover `[alias]:2225` line STOPS the bare lookup ssh actually performs — so the one population D2 cites HostKeyAlias for, bastions tunnelled through localhost:port, would get a hard failure on a host ssh connects to. So resolveKnownHostsLookupHost reports whether the name came from the alias, not just what it is, and that flag reaches both the matcher and the algorithm ordering. Returning the name alone is what made the bug invisible: the caller had no way to know it was holding something that must not be bracketed. Found by a parity review. 1,543 SSH tests pass. |
||
|
Neil
|
b54d078465 |
fix(ssh): read ssh2's host key algorithm list instead of copying it
ssh2 builds DEFAULT_SERVER_HOST_KEY at load time and prepends ssh-ed25519 only when a RUNTIME PROBE succeeds — it signs and verifies with a fixed Ed25519 key. On a build where that probe fails, ssh-ed25519 is absent from ssh2's SUPPORTED list too, and generateAlgorithmList throws `Unsupported algorithm: ssh-ed25519` from inside client.connect. That throw matches no retry classifier and no transport-fallback classifier, so it is permanent — and because we only set `algorithms` for hosts we already know, it would fire on trusted hosts while new ones kept working. A copied list cannot be merely stale here; it can be wrong. So it is read from ssh2 now, which also removes the drift risk the previous commit could only report. ssh2 is external in the main bundle and the bundle is CJS, so the deep path resolves at runtime from packaged node_modules. The copy stays as a fallback in case a future ssh2 moves the file — losing the proposal order degrades the type-scoping guarantee, but refusing to connect at all is worse. The test that used to pin the copy against ssh2 now pins the fallback, which is the only part that can still drift. Found by an availability review. pnpm lint clean; 1,537 SSH tests pass. |
||
|
Neil
|
38a1a7858a |
fix(ssh): name a ssh-keygen -R target that actually removes the entry
Verified against OpenSSH 10.2p1: with both `[h.example]:2222` and `h.example` on file, `ssh-keygen -R h.example` removes only the bare line and leaves the bracketed one — and there is no port flag, `-R host -p 2222` is "Too many arguments". An off-port target is keyed `[host]:port` in known_hosts, so the command we printed removed nothing: the user runs it, reconnects, and meets the identical failure with no indication of why. The message now names the bracketed form, quoted because the brackets are shell glob characters, whenever the port is not 22. Found by a parity review. |
||
|
Neil
|
1236ba11fb |
fix(ssh): refuse the known_hosts lines ssh itself refuses to parse
Three ways a line could be trusted by us and invisible to the user's own ssh — or, worse, raise a CHANGED alarm from an entry ssh drops. Buffer.from does not fail on bad base64, it SKIPS invalid characters, so `<key>!!!` and a blob with `@@` spliced into it both decoded to the correct key and matched. Verified live against OpenSSH 10.2p1 on 127.0.0.1:2224: the unmodified control reached authentication and all three malformed variants produced "No ED25519 host key is known". A re-encode-and-compare makes us agree. `<key>AAAA` is the interesting one, and the reason the first fix was not enough: 68 characters plus 4 is still legal base64, and the algorithm header still reads ssh-ed25519, so neither the base64 check nor the existing header check sees anything wrong. ssh parses the whole key structure. We decoded 54 bytes where an ed25519 key is 51 and reported `mismatch` — a man-in-the-middle warning caused by a typo in a file ssh silently ignores. So the blob is now walked as what it is: a run of length-prefixed fields that must consume it exactly. Algorithm-agnostic on purpose, so a key type we do not model is checked as well as one we do. It also rejects a length prefix that overruns the buffer, which readHostKeyType only checked for the first field. And ssh's extract_salt demands exactly one SHA1 digest — "expected salt len 20, got 16" — where we accepted any non-empty salt. A short salt is still a usable HMAC key for us, so a hand-crafted line could match for us and be a parse error for ssh. ssh-keygen -H always writes 20 bytes, so refusing loses no real entry. Worth recording that ssh-keygen -F cannot answer any of this: it matches host names and prints lines without ever decoding the key, so it reports "found" for all four blobs. The real client was the only instrument that worked. Found by a parity review; the base64 finding as reported was right about the behaviour and wrong about the mechanism for the padded case, which is what led to the structural check. 1,533 SSH tests pass. |
||
|
Neil
|
ef08403f80 |
fix(ssh): stop the fallback pass accepting a changed key, and refusing a new one
One wrong loop scope, two opposite errors, both reproduced against a live OpenSSH 10.2p1 client and an ed25519-only sshd on 127.0.0.1:2223. ACCEPTING A CHANGED KEY. ssh runs the bare-host fallback only when the port-qualified lookup matched no plain entry of ANY key type. We ran it unless pass 0 produced a match or a SAME-TYPE mismatch. So with an off-port RSA entry plus a bare, correct ed25519 line — an ordinary shape, an old off-port entry beside one written by a port-22 connect — ssh printed IDENTIFICATION HAS CHANGED and refused, with no "checking without port identifier" in -v because the fallback never ran, while we reached the bare line and returned `match`. REFUSING A NEW ONE. sawKnownHostOtherType and sawCertAuthority were declared outside the pass loop, so an entry found only on the fallback pass could set them. A bare ssh-rsa entry, dialed on a non-default port against an ed25519-only server, made ssh add the host and connect — plain first contact — where we returned unknown-type-known-host and hard-failed. That is Gitea/Forgejo, dev containers, Gerrit, Vagrant: an off-port service on a host already in known_hosts. So the flags are per-pass now, and pass 0 decides as soon as it finds any plain entry for the host. Which of the two rejections it reports only picks the message; ssh calls both HOST_CHANGED. Also drops the type check from the match test: byte equality already implies the types agree, because the blob carries its own algorithm name and parsing rejects any line whose declared type disagrees with it. Reverting either half of the scope fix fails exactly the two new tests. 1,526 SSH tests pass. |
||
|
Neil
|
b48c467978 |
fix(ssh): honour StrictHostKeyChecking, which had never once been read correctly
`ssh -G` does not echo the value the user wrote. StrictHostKeyChecking is rendered through fmt_multistate_int, which prints the first entry of multistate_strict_hostkey, and that table lists true/false before yes/no: yes -> true | no -> false | off -> false | accept-new -> accept-new | ask -> ask Verified against OpenSSH 10.2p1 from both a config file and -o. Not 10.2-specific; the table ordering is old. The decision function tested only 'yes'/'always' and 'no'/'off' — spellings that cannot arrive. So `StrictHostKeyChecking yes` fell through to the default branch and we accepted AND PERSISTED a host the user's config explicitly says to refuse. That is the worst outcome this feature can produce, and it was the behaviour for every strict user from the first commit. `no`/`off` landed there too, breaking the documented "lax settings never persist" invariant. Every unit test passed throughout, because they fed the function 'yes' — the value a human writes, not the one that reaches the code. My ssh -G parity test did capture real output, but I happened to configure accept-new, one of only two values that round-trip unchanged. The new table is keyed on configured value -> what ssh -G actually prints, and asserts both reach the same verdict, so the question "is this the spelling that arrives?" cannot be assumed again. Found by a parity review against a live OpenSSH client. |
||
|
Neil
|
2a9b57cf4c |
style(ssh): satisfy the code-quality lints in the two new test files
A string concatenation that should be a template literal, and an inline import() type annotation that should be a type-only namespace import — erased before vi.mock's hoisted factory runs, so the mock is unaffected. pnpm lint is clean. |
||
|
Neil
|
73132520de |
docs(ssh): record the two assumptions a live client confirmed
Both were load-bearing and neither was obvious: the bare-host fallback pass may not report a change, and unknown-type-known-host is what ssh itself does rather than something we invented. Getting the first backwards would have refused hosts ssh connects to happily, which is the failure mode that looks like caution. |
||
|
Neil
|
56cc790c7d |
test(ssh): check the matcher against a live OpenSSH client, not against my beliefs
Every other test in the parser file states what I believe ssh does. These state what it did: an OpenSSH 10.2p1 client against a real sshd on 127.0.0.1:2222, with the client's own verdict recorded from its output, and ssh-keygen -H's own salt and hash pinned as a vector. Two assumptions the design leans on were worth more than an argument. THE FALLBACK PASS MAY NOT REPORT A CHANGE. We look up `[host]:port` first and retry the bare host, and only the first pass may answer `mismatch`. With StrictHostKeyChecking=accept-new, a bare line holding a DIFFERENT key, dialed on 2222, ssh connected and appended a new `[127.0.0.1]:2222` line — no IDENTIFICATION HAS CHANGED banner. It read that as first contact. Had we reported a change there we would refuse hosts ssh connects to happily, and the wrongness would have been invisible: refusing looks like the cautious choice. AND THE TYPE-SCOPING REJECTION IS NOT AN INVENTION. known_hosts holding an ssh-rsa key while the server offers ed25519 makes ssh print IDENTIFICATION HAS CHANGED and refuse. So unknown-type-known-host is neither stricter nor laxer than ssh — treating it as first contact, which is what a naive type-scoped lookup does, is the laxer mistake. That second result also fixes the message. ssh is blocked too, so `ssh-keygen -R <host>` is the remedy that unblocks both, and we were naming it only for a same-type mismatch — leaving this case with a diagnosis and no way out. Named now when known_hosts is the source that disagrees, and still not named when it is our own store, which ssh-keygen would not touch. 1,516 SSH tests pass. |
||
|
Neil
|
62eb47a6ae |
refactor(ssh): give the store one matcher instead of two
The connect path had its own copy of the store comparison, because ssh2's verifier decides synchronously and cannot await the file, so records are preloaded. That copy is precisely where the type downgrade came from: it answered only match/mismatch/unknown, so a record of a different type for the same endpoint read as first contact and a host learned on first contact could be impersonated by presenting another key type. Fixing it left two implementations that have to agree forever, which is the same bug waiting to happen. matchTrustedHostKeys is now the single pure matcher; isTrusted is a load plus a call to it, and the connect path calls it directly on preloaded records. Same for the key types that feed the algorithm ordering, which the connect path was also filtering by hand. The two copies had in fact already drifted: the connect path lower-cased the query host where the write trims AND lower-cases. Not reachable today — the host is trimmed before it reaches there, which I confirmed by mutating the normalisation and watching the wiring test pass anyway. So this is not a bug fix, and the wiring-level test I first wrote for it proved nothing and is gone. The unit test that replaces it drives the matcher directly, where the input is mine to control, and it does fail when the normalisation diverges. Also adds an equivalence test across all six outcomes between the preloaded and awaited paths, so the two can never answer differently again. Restores the local name siteConfigSuppressed for the `-F` check; it was renamed along with the decision input, but at that site it really does mean only the one thing, and the union with the unreadable-file count happens one line later. 1,509 SSH tests pass. |
||
|
Neil
|
b71b007934 |
docs(ssh): record where the rejection message actually lands
Traced end to end, because a rejection the user cannot read is a half-shipped feature — and two of the surfaces were dropping it entirely, both now fixed. What is left is written down rather than guessed at: the status bar renders only 'Error', the terminal overlay's call to action still invites a retry that cannot succeed, toasts carry Electron's remote-method prefix, and the paired-web path replaces the text with 'SSH connection unavailable' on every route — which also affects a DESKTOP user viewing a host owned by a remote Orca server, not just web clients. |
||
|
Neil
|
088c058c9b |
fix(terminal): show why the SSH connection failed, not just that it did
The reconnect overlay took only a status, so every failure rendered the same sentence: "The SSH connection to devbox failed. Connect again to continue this terminal session." A refused host key and a network timeout were indistinguishable there, and the host key message — the only place that names the remedy, down to `ssh-keygen -R <host>` — reached no terminal user at all. The state carried it the whole way; the overlay simply never asked for it. Adds it as a second line rather than replacing the sentence. The sentence says what to do, the detail says what happened, and keeping both means an errno failure does not lose its guidance to make room for "connect ETIMEDOUT". Wrapped, for the same reason as the settings card: the remedy is at the end. Suppressed for a removed target, which already explains itself and can never reconnect — a stale connection error underneath would contradict it. The new selector mirrors selectRuntimeAwareSshStatus branch for branch, including the unreachable-environment and un-hydrated-bucket nulls, so the pair cannot disagree about which source they read and a detail is never shown next to a status it did not come from. Known and NOT addressed here: "Connect again" is still the wrong advice for a decision that will never change. Telling those apart needs a typed reason on the wire rather than a string, which is a remote-wire-compatibility decision; it is recorded in the STA-4319 follow-ups. Pre-existing on this branch and untouched: 2 failures in terminal-ime-xterm-resumed-preedit-visibility. |
||
|
Neil
|
ecc9df766e |
fix(settings): stop truncating the SSH connection error to one line
The host key messages are written to be actionable — a mismatch ends in "Run: ssh-keygen -R <host>", which is the remedy that also unblocks ssh and git. The only place in the renderer that displays an SSH connection error clamped it to a single line with `truncate` and carried no title attribute, so the remedy was unreachable, not even on hover. The careful wording reached a CSS ellipsis. Wraps instead, with [overflow-wrap:anywhere] because a long hostname offers no break opportunity and would overflow the column on its own. The paragraph only renders on failure, so the extra height costs nothing in the normal case. CORRECTION to the four preceding commits: they each claimed 3 pre-existing failures in ssh-connection-sftp-namespace.test.ts. That was my error — I had been running `npx vitest` without `--config config/vitest.config.ts`, so the project's setupFiles and execArgv were absent. Under the real config those 3 pass, and have throughout. The whole SSH suite is green: 1,504 passed, 13 skipped. Still failing on this branch and unrelated to it (different subsystems, no file overlap): 5 in terminal-snapshot-osc8-roundtrip and 2 in browser-cookie-import. |
||
|
Neil
|
fd277bb0cd |
test(ssh): cover the wire from an accepted key to a record on disk
Nothing covered the store end to end. Every connection test runs with it unwired — a real path, since it degrades to known_hosts only — so an accepted first-contact key was never observed becoming a record, and the record was never observed being believed on the next connection. Without that wire the store is dead weight: an unknown host connects every time and is never learned. Its own file for two reasons. initSshHostKeyStoreFile binds module-level state for the rest of the process, and binding it makes the connect prelude do real disk I/O — which the shared suite cannot absorb, because its reconnect tests drive the clock with fake timers and an fs round trip does not complete inside an advanced tick. Adding these there turned 12 unrelated tests red. Covers: the record is written; it is read back as a match; the same key is not recorded twice; a DIFFERENT key for a host we recorded ourselves is refused (the store's entire security value, and a case known_hosts cannot catch since it has never heard of the host); and an on-demand runtime records nothing. Stubbing rememberHostKey to a no-op fails four of the five. |
||
|
Neil
|
f2070df6db |
test(ssh): pin the parser against real ssh -G output, and accept-new against ask
Two gaps the audit named. The ssh -G fixtures were all hand-written, which means they encode what I expect ssh to print. This one is verbatim OpenSSH_10.2p1 output for a Host block using HostName, HostKeyAlias, StrictHostKeyChecking accept-new, two UserKnownHostsFile paths and a non-default port. The format detail that matters: each file list arrives space-separated on ONE line, so reading it as a single path would consult nothing for anyone with more than one file configured. And accept-new was untested despite being a real OpenSSH value. It currently behaves identically to ask, which is exactly right while no dialog exists and is what lets the defence ship without a modal — so the equivalence is now pinned, and Phase 2 has to break it deliberately rather than discover it. Plus a guard that accept-new never falls into the strict branch. |
||
|
Neil
|
7c788020e5 |
test(ssh): pin the ephemeral carve-out where it is actually observable
The first version of this test asserted an on-demand runtime connects on first contact, which every target does — it would have passed with the carve-out deleted. Pointing HOME at a home whose known_hosts exists and will not open makes the two cases diverge: a normal target is refused there, an on-demand runtime is not. Removing the carve-out now fails three tests instead of none. Also covers the wiring for the unreadable-source refusal itself, which until now was only pinned at the decision level. |
||
|
Neil
|
efcb407c89 |
docs(ssh): record what review changed and the STA-4319 follow-ups
The design survived implementation; every defect found afterwards was in the wiring. Worth recording the pattern, because it repeated five times: each one made us either blind or unusable, never subtly wrong — and four of the five broke legitimate hosts rather than admitting bad ones. Action items separate the things Phase 2 must decide (UpdateHostKeys, which is now the likeliest way a legitimate user meets a rejection; the web client never seeing the reason; RPC fail-fast) from the gaps Phase 1 knowingly accepts (WSL, CheckHostIP, ca-only hosts, the hand-copied ssh2 algorithm list). Also flags the rollout risk plainly: this is the first release in which Orca can refuse an SSH connection at all. |
||
|
Neil
|
03264f2d82 |
fix(ssh): let an ephemeral runtime outrank sources we could not read
Three things, all about the same question: when we cannot see everything that decides a host key, what does that actually license us to refuse? 1. ON-DEMAND RUNTIMES WERE REFUSED FOR A POLICY THEY COULD NEVER SATISFY. A machine provisioned a minute ago cannot be in known_hosts, by construction — which is why it has a carve-out at all. But the carve-out sat BELOW the incomplete-sources check, so anyone whose HOME diverges from their passwd home (sandboxes, our own E2E isolation) took the `-F` path, and every on-demand runtime connection was refused, pointing at a config file the user cannot fix. Refusing there buys nothing. No policy, seen or unseen, is satisfiable by a host that did not exist yesterday; the trust comes from the provisioning channel. So the carve-out now outranks it. An EXPLICIT StrictHostKeyChecking=yes still wins over both — that one we can read, and the user asked for it. 2. THE FLAG WAS NAMED FOR ONE OF ITS TWO MEANINGS. `siteConfigSuppressed` started as "-F hid /etc/ssh/ssh_config" and had since grown "a known_hosts file exists and would not open" — which is not a site config, and reading the name in the decision function told you nothing about why an unreadable file landed there. It is `verificationSourcesIncomplete` now: we could not see something that decides this, so do not extend NEW trust. A host we already know still connects, because a match is decided before this is reached, and that is now pinned by name. 3. THE COPIED ssh2 ALGORITHM LIST HAD NO DRIFT ALARM. We reorder ssh2's default host-key proposal, which meant hand-copying a list ssh2 exports only from a deep internal path. ssh2 throws `Unsupported algorithm` on anything outside its supported list, so drift does not degrade — every target stops connecting, before a socket opens, with a message about an algorithm the user never chose. Worth knowing that the list is also built conditionally on ed25519 support. Kept as a literal rather than a deep import, since silently adopting a new proposal order is the wrong default — the order is what makes type-scoped matching safe, so a change deserves review. A test now compares it against ssh2's real constant and checks every entry is one ssh2 accepts. Moved next to the ordering function it feeds, and out from between the import statements. 1,488 SSH tests pass. The 3 in ssh-connection-sftp-namespace.test.ts are pre-existing on origin/main. |
||
|
Neil
|
6dcfcdface |
fix(ssh): stop treating an absent known_hosts as a source we failed to read
The previous commit's "no readable source is not first contact" guard was right about the danger and wrong about how to detect it, and the version that shipped would have refused every connection a new profile ever makes. A file that does not exist and a file that refuses to open both arrive as a rejected readFile, and I counted them the same way. They are opposites. An absent known_hosts is the normal state — ssh creates it on its own first connect, and an Orca profile that has never connected has none — and it is real evidence that no host is known. A file that exists and will not open is evidence withheld: the entry that would have said "this key changed" may be sitting in it. The default list is the reason this was fatal rather than obscure. It always names known_hosts2, which essentially never exists, so on a machine with a normal known_hosts the count was 1-of-2 and everything worked; on a fresh profile it was 0-of-2 and every connect failed with "the system SSH configuration could not be read" — a message about a file the user does not have and an error they cannot act on. The suite passed only because the machine running it happens to have a known_hosts. Pointing HOME at an empty directory fails 62 connection tests, which is what the new wiring test does. So the count is now unreadableFileCount: files that exist and could not be read, which is the condition the guard was always trying to express. Any one of them takes the strict path; an absent or empty file takes none. The store clause is gone with it — a store hit already returns accept before this is consulted, so it never changed an outcome. Tests: absent, empty, permission-denied, directory, and all-parsed at the source; first contact with no known_hosts at all at the wiring level. 1,482 SSH tests pass. The 3 failures in ssh-connection-sftp-namespace.test.ts are pre-existing on origin/main and untouched here. |
||
|
Neil
|
5839ac2d84 |
fix(ssh): stop offering credentials to a host we just refused
A refused host key ended the handshake and then fell into the credential ladder, because ssh2 reports a denied key as a generic auth failure and the passphrase branch is eligible on message shape alone whenever an encrypted identity file is configured. So the sequence was: decide this host may not be who it claims to be, then ask the user for their passphrase and hand it to it. Failing that, prompt for a password. Failing that, retry over the system ssh binary, which for a disagreement with our own store rather than known_hosts would simply connect. That inverts the point of checking at all. A denied key is now final for the attempt: recognised by type before any fallback runs, and again inside the agent-fallback retry, which re-runs the handshake and so can be the attempt that denies. Two things had to change for that to hold. The rejection is now a HostKeyVerificationError rather than a rebuilt Error, so the connect path recognises it by type. Substring matching would have worked today and quietly stopped working the first time a reason string was reworded — and these strings are already worded around the auth-error classifier, so they are exactly the kind that get edited. And the verifier now reports the denials that skipped the policy: an unreadable key blob and an internal failure both denied without calling onDecision, so the connect path saw only ssh2's generic failure and walked the ladder. That was the actual path the new test hit first. The report carries no fingerprint, since there is no host key to identify, and the connection no longer overwrites the fingerprint it holds with an empty one — the relay keys install-lock isolation on that value. The reconnect ladder also refuses to retry it. Its classifier is otherwise substring-driven, so a reason containing "connection reset" would have been retried until the ladder gave up, burying the reason under nine attempts. Tests: no credential prompt after a refusal, an encrypted key configured so the passphrase branch is eligible; refused reports as 'error', not 'auth-failed', which would invite the user to re-enter credentials that are not the problem; no retry; both bypassing denials report; a throwing listener still denies rather than hanging the handshake. Also drops three test files a bisect resurrected from before the revert that deleted them. 172 tests pass across the three touched files; typecheck and lint clean. Pre-existing on origin/main and untouched here: 3 failures in ssh-connection-sftp-namespace.test.ts. |
||
|
Neil
|
11b2f93cf8 |
fix(ssh): close two downgrades the implementation review found
Both were in my wiring, not the design, and two reviewers found the first independently. 1. OUR STORE WAS TYPE-DOWNGRADABLE. The inline lookup filtered by key type first and could only answer match/mismatch/unknown, so a record of a DIFFERENT type for the same endpoint read as "unknown". A host learned on first contact — ed25519, since ssh2 proposes it first — and absent from known_hosts could then be impersonated by presenting RSA: both sources say unknown, so accept-and-remember, silently. That is exactly the downgrade D3 says the design cannot ship without, applied to the records we create ourselves. The store's own isTrusted already computed the right answer and had no production caller. Stored types now also feed the algorithm ordering, without which the guard is only half present. 2. WE KEYED ON THE ORCA LABEL, NOT THE DIALED HOST. "ssh -G" echoes its own argument back as its hostname field when no Host block matches, so for a manual target that field IS the Orca label — the one name D2 forbids keying on, and one ssh never wrote. We consulted no entries at all, so an impersonated host read as first contact. Now keys on the dialed host, which buildConnectConfig has already resolved through HostName, with HostKeyAlias still winning. That inverted an existing test rather than deleting it: "uses the resolved hostname, never the Orca label" encoded an assumption disproved against OpenSSH 10.2p1, so it is renamed and reversed with the reason recorded in the test. 3. NO READABLE SOURCE IS NOT FIRST CONTACT. Every known_hosts file failing to read was indistinguishable from "this host is unknown", so a changed key would be accepted the one time we could not check. The loader now reports how many files it could read, and zero readable sources with an empty store takes the strict path instead of recording trust. 4. A superseded attempt's rejection could replace the live attempt's error, and substituting a new Error drops ssh2's code, so a transient ECONNRESET would stop being classified as retryable. The rejection is now local to its attempt. 5. displayHost was the Orca label, so a mismatch could print "ssh-keygen -R <label>" — a remedy that removes nothing. Also adds the tests that would have caught 1 and 2, the stale-attempt denial, and IPv6 literals, which the design moved into scope and nothing covered. 1,469 SSH tests pass. |
||
|
Neil
|
0f37005de0 |
fix(ssh): bind the host key store to the active profile at startup
Without this the store reports nothing trusted on every launch. Safe — known_hosts still decides, and a host trusted only by us degrades to first contact and is re-recorded — but it silently discarded our own accept records, so the store the design calls for was not actually in use. Bound beside the profile Store, since it is a sidecar of the same data file. Also records why the paired-web carve-out the migration review asked for is NOT implemented in Phase 1, rather than leaving it looking forgotten. That carve-out exists to stop a web client waiting out the 120s prompt timeout — a hang only reachable if a prompt exists, and Phase 1 has none, which the decision function pins with a test asserting it never returns 'prompt'. An RPC connect therefore behaves exactly like a local one. Adding a fail-fast path now would introduce a failure mode for a hang that cannot occur; it becomes load-bearing when the dialog lands and is listed under Phase 2. Noted there for whoever builds Phase 2: runtime/rpc/methods/ssh.ts already swallows the specific error and rethrows a generic one, so the host-key reason will not reach a web user without a change there too. Full unit suite: 52,352 pass. The 8 failures are the known environment baseline (5 osc8, 2 IME) plus one browser-cookie suite-ordering flake that passes in isolation — none in src/main/ssh, and none related to this change. |
||
|
Neil
|
a1331d2602 |
fix(ssh): verify host keys instead of accepting every one (STA-4319)
The actual fix. ssh-connection's verifier recorded a fingerprint and returned
true, so every ssh2 connection accepted every host key — no known_hosts consult,
no change detection. It now consults the user's known_hosts plus our own store
and refuses a changed, revoked or unverifiable key.
Phase 1 by design: no dialog. Unknown hosts are accepted and recorded
(accept-new semantics), because startup restore opens every previously-active
target at once, ephemeral VM targets present a new key each launch, and
paired-web connects run on the host desktop where a prompt would appear on
someone else's screen. The MITM defence lands now; the prompt is Phase 2.
Also sets algorithms.serverHostKey to lead with the types already known for the
host. Without it the type-scoped matching is a downgrade — an attacker who cannot
forge the key on file just presents another type and turns a hard failure into
first contact. Verified from ssh2's kex.js that the client list decides.
Denial replaces ssh2's generic handshake error with the specific reason, because
the reconnect ladder cannot distinguish a generic failure from a transient fault
and would retry forever against a decision that will never change.
An unreadable trust store degrades to known_hosts only rather than failing the
connect: a changed key is still refused, and a host trusted only by us falls back
to first contact and is re-recorded, reaching the same decision.
The ssh2 mock now uses the callback form and aborts the handshake on denial. As
written it called hostVerifier(key) with one argument and ignored the result, so
it would have passed against a verifier that never decides — flagged in the
design as a mock that had to change, not a test to quietly rewrite. Two new tests
pin the wiring rather than the module: an unidentifiable blob is refused, and a
well-formed key is accepted.
Note for review: commit
|
||
|
Neil
|
43ed05509b |
fix(ssh): promote every RSA signature algorithm for a known ssh-rsa key
A known_hosts entry names the KEY type, which is not the negotiated ALGORITHM name. One ssh-rsa key is offered as rsa-sha2-512, rsa-sha2-256 or ssh-rsa depending on the signature algorithm, so matching the literal name only would leave a host we know by RSA ordered behind ed25519 — precisely the ordering this function exists to prevent, and precisely the population (RSA-era known_hosts entries) it was written for. Verified from ssh2's own negotiation while wiring this: kex.js iterates the CLIENT list and takes the first entry the server also offers, so client order does decide, as RFC 4253 says. ssh2's default order leads with ed25519 and places the RSA algorithms fifth through seventh. |
||
|
Neil
|
2d2a0880ba |
feat(ssh): build the host key verifier and the algorithm order that makes it safe
Still not wired into the handshake — that lands next. This is the piece that turns a decision into an ssh2 callback, plus the half of the design that is easy to forget because it lives in a different config field. The verifier MUST be a plain function returning undefined. ssh2 does 'const ret = verifier(key, verify); if (ret !== undefined) verify(ret)', so an async function returns a Promise — neither undefined nor falsy — and ssh2 accepts the key immediately while ignoring whatever the callback later decides. Making this async would silently restore exactly the accept-everything behaviour the module exists to remove, so a test asserts the return value is undefined. orderServerHostKeyAlgorithms is what makes type-scoped matching safe rather than a downgrade. RFC 4253 gives the client's algorithm order priority, so leading with the types we already hold for a host denies a server the choice of presenting some other type to convert a hard failure into first contact. Without it, an attacker who cannot forge the key on file just offers a different algorithm. Revoked entries never contribute to that order. Also fails closed on two paths that would otherwise hang or over-trust: a key whose own length-prefixed header cannot be read is refused rather than reasoned about, and a throw from any dependency denies, because ssh2 may not catch an exception raised inside the verifier and the handshake would hang instead of failing. 18 tests. Includes the two negative cases that matter — first-contact keys are recorded, but keys we already know, rejected keys, ephemeral runtime targets and a lax StrictHostKeyChecking are not. |
||
|
Neil
|
b5e27ab7b7 |
feat(ssh): decide what to do with a presented host key
The policy half of STA-4319, kept separate from the ssh2 wiring so it is testable without a handshake and injected rather than importing its sources, so a test states its own trust state instead of writing files. Phase 1 ships no dialog — a test asserts the decision is never 'prompt'. Startup restore opens every previously-active target at once, ephemeral VM targets would ask every launch, and paired-web connects run on the host desktop where the dialog would appear on someone else's screen. Ordering that matters: revocation outranks everything including StrictHostKeyChecking=no, because a revoked key is a statement that this key is known-bad rather than merely unrecognised. known_hosts is named before our own store on a change, because its remedy (ssh-keygen -R) is the one that also unblocks ssh and git — pointing at a remedy that cannot work is worse than none. Two carve-outs with reasons: an ephemeral runtime target accepts WITHOUT recording, since a fresh VM presents a new key every launch and a stored record would accumulate per launch and eventually read as a spurious change; and when ssh -G ran on the HOME-divergent path that suppresses /etc/ssh/ssh_config, an unknown host is denied, because a site-wide policy may forbid it and being laxer than ssh is the one outcome that is never acceptable. Rejection text deliberately avoids 'authentication failed' and 'permission denied': the reconnect ladder classifies on those substrings, so a denial phrased that way is retried forever against a decision that will never change. Pinned by a test. |
||
|
Neil
|
bee19b083f |
feat(ssh): parse and match OpenSSH known_hosts
The matcher half of STA-4319. No behaviour change yet — nothing calls this.
Hand-rolled because no maintained JS implementation exists, and written against
behaviour observed from OpenSSH 10.2p1 rather than inferred from the man page.
Three of those behaviours a reasonable reading gets wrong:
- A non-default port is TWO ordered lookups, not one candidate set: '[host]:port'
first, then bare host ('checking without port identifier' in ssh -v). The
fallback pass can only yield match or unknown — OpenSSH downgrades a wrong key
there rather than reporting a change. Collapse them and anyone holding a bare
line who connects off-port gets a spurious first-contact result; treat the
fallback as authoritative and they get a false change-of-key alarm.
- Revocation resolves in its own pass so the verdict cannot depend on line order.
Verified both orderings.
- A cert-authority line never matches a plain host key; it only validates
certificates. A normal line alongside it still decides.
Mismatch is scoped to the same key type, and a host known by a DIFFERENT type
returns unknown-type-known-host rather than plain unknown — an attacker who
cannot forge the key on file must not get a friendly first-contact result by
presenting another type. That outcome is only half the defence; the other half
(leading serverHostKey with known types) lands with the wiring.
47 tests from vectors executed against real sshd, including ssh-keygen -H hashed
entries. Each of six mutations reddens it: collapsing the passes, letting the
fallback report mismatch, dropping type scoping, resolving revocation in line
order, honouring an unrecognised marker, and skipping the blob/type agreement
check.
|
||
|
Neil
|
8be1696629 |
docs(ssh): revise the host key design after security and migration review
Three things the reviews changed, kept visible rather than quietly edited out. THREAT MODEL WAS WRONG IN THREE PLACES. Jump hosts are not the worst case — they are already safe: shouldUseSystemSshTransport branches on exactly the inputs resolveEffectiveProxy does, and attemptConnect returns after the system probe, so ProxyJump goes through OpenSSH and is verified. Agent forwarding was overstated (gated on the user's ForwardAgent). Credential theft was understated: any auth error counts as agent fallback, so a MITM walks the user to the password AND private-key passphrase prompts, and cachedPassword replays without prompting. The relay claim was backwards — the attacker owns their own machine; the real impact is the return direction, where they become the host our workspace trusts. TYPE SCOPING IS A DOWNGRADE VECTOR WITHOUT ALGORITHM ORDERING. This was the decision I flagged as least certain and asked to have argued both ways. OpenSSH is safe only because order_hostkeyalgs() puts known types first and RFC 4253 gives the client's order priority. ssh2 negotiates ed25519 first regardless, so an attacker who cannot forge the RSA key on file just presents ed25519 and gets a friendly first-contact prompt instead of a hard failure. Keep scoping, but set algorithms.serverHostKey to lead with the types on file — and add a sixth outcome for 'unknown type, known host', which must never read as first contact. SHIP THE DEFENCE BEFORE THE DIALOG. Startup restore fires eager connects for all targets in parallel with a 15s timeout while a prompt would live 120s; ephemeral VM targets present a new key every launch; paired-web connects run on the host desktop, so the dialog opens on someone else's screen. Phase 1 is therefore no modal at all: consult known_hosts and our store, match connects, unknown persists with accept-new semantics, mismatch and revoked hard-fail. That is the whole MITM defence with none of the migration risk. Also folded in, verified live against OpenSSH 10.2p1: the without-port fallback (bracketed lookup first, then bare, where the second pass can only yield match or unknown — otherwise a bare line plus a non-default port produces a spurious prompt); hashed entries hash the candidate form; multiple files union; a cert-authority line does not match a plain key. IPv6 and bracket parsing moved INTO scope — that is a parser requirement, not a scope call, and getting it wrong produces the prompt-training harm the design exists to avoid. |
||
|
Neil
|
2a99a95b9c |
docs(ssh): design for real host key verification (STA-4319)
Today's ssh2 verifier records a fingerprint and returns true — every host key is accepted, with no known_hosts consult and no change detection anywhere in src/main/ssh/. Scope is per-connection, so exec, SFTP, port forwarding, the watcher and relay deploy all ride that one unverified handshake, and the ProxyJump path puts the final hop — the topology most likely to cross untrusted network — on ssh2 specifically. Decisions worth calling out: - Read the user's known_hosts as a trust source but NEVER write to it. That file is shared with every other SSH tool on the machine; appending means line endings, permissions, concurrent writers and a corruption blast radius well beyond us. Accepted keys go to our own per-target store. Reading theirs is also the entire migration story: most developers already have their hosts there. - Mismatch is scoped to the SAME key type. A host with only an RSA entry that presents ed25519 is unknown, not changed. ssh2 negotiates ed25519 first, so without this we would fire a change-of-key alarm at nearly every existing user on their first upgraded connect — training them to dismiss the one warning that is supposed to mean something. Flagged in review as the decision I am least sure of; a downgrade-vector argument against it is being tested. - Changed key hard-fails with no override button; recovery is a separate explicit action, offered only when OUR store is what disagreed, because forgetting our record cannot unblock a known_hosts conflict. - Background reconnects deny rather than prompt. A dialog the user cannot place in context only teaches click-through. Two traps are documented because either would make the fix silently do nothing: an async verifier returns a Promise, which ssh2 reads as truthy and accepts immediately; and the existing test mock invokes hostVerifier with one argument and ignores the return, so it would pass against a verifier that never decides. Design only — no behaviour change. The doc is added to the tracked-reference allowlist in .gitignore alongside the other docs/reference entries. |
||
 Jinjing
|
cb42b60849 |
fix(linear): label Start workspace and add Open on Linear (#14492)
* fix(linear): label Start workspace and add Open on Linear The issue page header used three unlabeled icons. Match the GitHub issue header: copy stays quiet, Open on Linear is an external-link control, and Start workspace is a labeled primary button. * Address PR review feedback (#14492) - Make the header source contract ignore Start workspace formatting |
||
|
Neil
|
87bafb5e6a |
fix(terminal): ground the emulator to the snapshot's baseline after a byte gap (#14374)
A hidden-delivery byte gap can strand more than the SGR pen, and the reset #14241 added to the split alt-screen replay is undone before any content is painted: xterm answers `?1049l` with restoreCursor(), which reloads the pen, all four G-set designations, GL, origin mode and wraparound from the register saved at `?1049h`. - Bracket the buffer switch with the baseline: before, so `?1049h` banks grounded state rather than the gap's; after, so `?1049l`'s restore cannot reapply it. - Ground everything a serialized payload is diffed against, not just the pen: SGR, GL, all four G-sets, origin, autowrap, insert, the per-buffer scroll region, and the saved-cursor register. - Switch buffers only when the pane is actually on the other one. `?1049` is not a no-op otherwise — it still swaps the kitty flag registers, which would park the flags of an agent that negotiated them on the normal screen. - Return to the normal buffer when the gap ate the TUI's exit sequence; the restored history was painting into the alt buffer with scrollback left empty. - Restore the CAN #14241 dropped, so a control string the gap truncated is discarded instead of committed by the next ESC. - Ground the abandon path exactly once instead of twice. - Derive the parity/fuzz preambles from the same builder; they had drifted and were asserting against bytes production no longer emits. |
||
 NeilandOrca
|
41ba29d79a |
test(e2e): headless preedit-geometry coverage for Korean and CJK terminal input (#14500)
* test(e2e): headless preedit-geometry coverage for Korean and CJK terminal input Both IME defects that shipped and were reverted passed a suite of ~3000 IME assertions, because every one of them checked bytes reaching the pty and a preedit rendered into a hidden overlay satisfies all of them while the user composes blind. The one arm that asserted real geometry was headful-gated and macOS-only, so it never ran in CI. Drives composition through CDP Input.imeSetComposition rather than a native input source, which removes the accessibility grant, the system input source and the visible window that forced that gate, so this runs in the ordinary electron-headless project. The load-bearing assertion is the composition overlay's real bounding rect. Verified to have teeth: with max-width 0 and overflow hidden injected, the active class, the textContent, display block and checkVisibility all still pass, and only the rect assertion fails. * test(e2e): restore the CDP composition drivers the preedit specs need The trimmed copy on main kept only the key-dispatch helpers, so the composition drivers the geometry specs import were missing. Adds them back: setImeComposition, commitImeText, dispatchImeProcessKey, composeHangulSyllable and dispatchResumedCompositionUpdate. The shared helpers are unchanged. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai> |
||
  Langning ZhangandJinwoo-H
|
92b6ffd17d |
Terminate renderer graph reload generations and contain disposed-frame notifications (#14070)
* fix(runtime): terminate renderer graph reload generations * fix(runtime): harden renderer reload teardown * fix(runtime): fence renderer graph publication ownership * test(runtime): register renderer graph reload gate * test(runtime): record live reload validation * fix(runtime): ignore cancelled renderer navigations * chore: preserve main formatting during branch sync * chore: satisfy changed-code quality gate * fix(runtime): restore cancelled renderer reloads * fix(runtime): preserve committed reload fencing * test(runtime): prove cancelled reload timeout * docs(reliability): record reload cancellation oracle --------- Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> |
||
|
NeilandOrca
|
46e1c3eee4 |
fix(terminal): strip the captured shim dir across trailing-separator spellings (#14439)
* fix(terminal): strip the captured shim dir across trailing-separator spellings The scrub compared the captured ORCA_ATTRIBUTION_SHIM_DIR to PATH entries literally, so a trailing-separator difference left the legacy shim directory on the spawned PATH. Same class already fixed in the generated wrappers. Also drops a dead default in the POSIX filter, consolidates comments that had accumulated across fixes, and normalizes the legacy directory once rather than per PATH entry in the cmd wrapper. The boundary scanner stays: a shim path can contain the PATH delimiter, which splitting would fragment. * fix(terminal): keep the git shim tombstone parseable on Windows The cmd wrapper carried two em dashes in comments. cmd.exe seeks through a batch file in bytes but advances by decoded character count, so those four extra UTF-8 bytes made it drop the first four characters of every line and the wrapper died with "The syntax of the command is incorrect." Also move the legacy-dir trailing-separator strip into a CALL body: cmd expands a whole line before evaluating `if defined`, so inline it ran its substring syntax against an unset variable and mangled the line. Rooted-path checks are shared by a single subroutine, a relative or drive-relative captured ORCA_REAL_* is rejected, and relative PATH entries are dropped from the exported PATH so the cwd cannot select spawned tools. The POSIX tombstone is deleted rather than written when no absolute interpreter can be verified. Verified on Windows 11 (cmd and PowerShell 5.1): normal lookup, relative and drive-relative ORCA_REAL_GIT, relative and drive-relative PATH entries, trailing separators, legacy shim dir, empty PATH, and a cwd-only PATH with a planted git.cmd/git.bat. Co-authored-by: Orca <help@stably.ai> * fix(terminal): stop the git shim tombstone re-expanding PATH data cmd re-expands a CALL command line, so path data handed to a subroutine as an argument got a second round of percent expansion. A PATH entry holding a literal %CD% became the current directory before the rooted-path guard saw it, and the wrapper then ran a planted git.cmd from that directory (exit 66, reproduced on Windows 11). Callers now pass the value in a variable, which is expanded once. Re-verified on Windows 11 across 15 cases: the %CD% entry is now dropped and the real git runs, and a PATH entry spelled 'C:\paren9 (x86)\cmd' still resolves, so the new for-block did not regress paths with parentheses. Co-authored-by: Orca <help@stably.ai> * fix(terminal): keep percent expressions out of the shim tombstone comments cmd expands variables inside rem, so a comment naming the working directory substituted a path into itself. Verified on Windows 11 that rem does not re-parse the result -- a cwd of 'C:\x&pwned&rem' executed nothing and the real git still ran -- so this was not exploitable, but rem handles separators differently inside a parenthesized block and this script now has some. A test now rejects any percent sign in an emitted rem line. Co-authored-by: Orca <help@stably.ai> * fix(terminal): pin shim tombstone shell state and directory identity Three fixes, each proven before and after. Delayed expansion: bare setlocal inherits the caller's state. Under a parent shell started with /V:ON, a literal !CD! PATH entry became the current directory and a planted git.cmd ran (exit 66), and a legitimate directory containing ! stopped resolving (exit 127). Both on Windows 11, both gone with setlocal DisableDelayedExpansion. Directory identity: the POSIX filter compared the legacy shim directory lexically while comparing its own directory with -ef, so a symlink or a <legacy>/../<legacy> spelling escaped the filter and the live attribution wrapper won the lookup. It now tests both. The env scrub had the same gap and now normalizes before its suffix test. Retained POSIX wrappers: with no absolute bash verifiable the wrapper was deleted, which strands a shell that already hashed the path on 127 instead of falling through to PATH. It now reuses the shebang of the wrapper it replaces, which is known to work on that host, and rejects /usr/bin/env so the ambient lookup stays closed. Deleting is the last resort. Two Windows test pins matched the wrong occurrence and stayed green with the guard they claimed to protect removed; they now assert the subroutine body. All five fixes were mutation-tested. Co-authored-by: Orca <help@stably.ai> * fix(terminal): require bash for a reused shebang and exclude slash-spelled dirs The retained-shebang fallback accepted any absolute executable that was not env, but the rendered body needs BASH_SOURCE, [[ and local, so a #!/bin/zsh wrapper was accepted and then exited 1 on 'BASH_SOURCE[0]: parameter not set'. It now requires bash, which also rejects /usr/bin/env as before. The PowerShell filter trimmed only backslashes while its rooted-path regex accepts forward slashes, so a wrapper or legacy directory spelled with a trailing / missed the lexical exclusion. Verified on Windows 11 that both spellings are now excluded and the real git still runs. The test that claimed to cover the shebang fallback only called the resolver directly, so deleting the wiring left the suite green on any host with /bin/bash. It now mocks the resolver to null and asserts through neutralizeLegacyTerminalShimDir that the wrapper survives with the retained shebang, and that a wrapper without a reusable one is still deleted. Both mutations are now killed. The Windows wrapper text assertions move to their own file rather than taking a max-lines exemption. Co-authored-by: Orca <help@stably.ai> * fix(terminal): stop the shim PATH scrub deleting a legitimate directory The previous round collapsed '..' lexically before classifying a PATH entry. That is not the same as resolving it: when <shim>/posix is a symlink, <shim>/posix/../posix lands elsewhere, so a legitimate directory was classified as the shim and removed, leaving git unresolvable. Reproduced with a symlinked shim/posix and a real git behind it. Resolving for real is not available here either -- this env is also built for remote and WSL panes whose paths name nothing on the local filesystem -- so the classifier is lexical again, deliberately. A '..' spelling that slips through costs nothing at runtime: that directory holds the pass-through tombstone, and the tombstone excludes its own directory by -ef, so the lookup still reaches the real git. Separately, pathEntrySpellings can only enumerate one added separator, so a captured directory spelled with two or more survived the literal removal. The split filter now also compares separator-stripped forms, which covers any number. Both changes are mutation-tested. Co-authored-by: Orca <help@stably.ai> * fix(terminal): stop a relative shim dir letting the cwd pick the binary The -ef identity test added two rounds ago resolves a relative right-hand operand against the wrapper's current directory, so a relative ORCA_ATTRIBUTION_SHIM_DIR let the cwd decide which PATH entry counted as the legacy directory and got a legitimate one skipped. Reproduced as SAFE vs LATER purely by changing the cwd. Identity is now attempted only for an absolute target; the lexical compare still covers the rest. The cmd wrapper had the same shape: full-path expansion made a relative captured value absolute against the cwd before PATH filtering. It now requires a rooted value and leaves the normalized form unset otherwise, which makes the reject subroutine a no-op. Verified on Windows 11 that two runs differing only in cwd now agree. Separately, trailing-separator stripping treated a backslash as a separator on POSIX, where it is a legal filename character, so '/tmp/captured\' and '/tmp/captured' compared equal and a real directory was deleted from PATH. The rule is platform-specific now; the cross-platform classifier still understands both styles because a Windows PATH reaches it through the remote env. Both fixes are mutation-tested. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai> |
||
|
Neil
|
b82a8791f5 |
perf(runtime): resolve an explicit worktree id without scanning every repo (#14399)
`resolveWorktreeSelector` resolved every selector kind from the whole-fleet snapshot, so a targeted `id:<repoId>::<path>` lookup fanned `git worktree list` across every registered repo to answer a question about one of them. With a cold scan cache -- app startup, or the first lookup after a mutation clears the snapshot -- that is one subprocess per repo, ~17ms each, to find a worktree whose owning repo the id already names. Measured on a ten-repo fleet: one `id:` lookup scans 10 repos before and 1 after. Scope only `id:`. Every other selector kind is matched across the fleet and its `selector_ambiguous` contract is defined over all repos, so scoping `branch:`, `name:`, `issue:`, or a bare selector would silently pick a winner where they correctly refuse today. A test pins that: `branch:main` across ten repos still throws `selector_ambiguous` and still scans all ten. Lineage stays correct because edges are intra-repo by construction. The scoped path returns null and falls back whenever that does not hold: a repo id registered on several execution hosts, an unknown repo id, or a worktree the scoped scan does not contain. A warm fleet snapshot always wins. Row resolution moves out of orca-runtime.ts into repo-worktree-row-resolution.ts, which owns no state -- the cache-aware scan and folder-workspace stamping are injected. orca-runtime.ts ends up 65 lines shorter than before despite the added feature. |
||
|
Jinjing
|
a6a64439a0 |
fix(terminal): keep split error when rejected cleanup throws (#14463)
Wrap kill and retireRejectedPty so a cleanup failure cannot replace the original split-authority error or skip the remaining teardown. |
||
 dngur6344
|
78ca45e2ae |
fix(renderer): remove duplicate git history tooltips (#14453)
* fix(renderer): remove duplicate git history tooltips * test(renderer): harden tooltip regression coverage |
||
|
Jinjing
|
d03eb3218e |
Revert "fix(renderer): stop git history hover from showing two tooltips (#14468)" (#14485)
This reverts commit
|
||
|
NeilandOrca
|
2100fb2553 |
fix(runtime): cap remote git.diff and file previews at the transport budget (#14160)
* fix(runtime): cap remote git.diff and file previews at the transport budget A remote or mobile user who opens the diff of a large image loses their whole WebSocket, not just that request: the E2EE channel closes with 1013 when a reply exceeds the 4 MiB outbound envelope. Two producers can exceed it unaided. git.diff/branchDiff/commitDiff cap text with MAX_RENDERED_DIFF_COMBINED_CHARACTERS (6M chars) -- a *renderer* budget that sits above the transport limit -- and return base64 for previewable binaries bounded only by MAX_GIT_SHOW_BYTES, so a 10 MiB PNG changed in place is ~26.7 MiB in one envelope. files.readPreview inlines base64 up to 10 MiB, and mobile calls it for every image tab. Both now measure against a budget derived from the outbound limit. The check sits in orca-runtime-git.ts, downstream of the dedupe and of both the SSH-provider and local branches, so a payload forwarded verbatim by an old relay is covered by the same code and src/relay needs no change. Local and in-process callers pass no budget and keep full fidelity. Measuring raw bytes would not work, which is the whole reason this needs a module. JSON escaping turns one control byte into six (\u00XX), and binary-buffer.ts sniffs only for NUL in the first 8 KiB -- so a NUL-free file of 0x01-0x1f bytes is classified as *text*, would pass a raw-byte cap, and would then blow the envelope. The budget is escape-aware, with a three-branch fast path that keeps normal diffs at two native byteLength calls and scans only the ambiguous band. The SSH branch of readFileExplorerPreview had the same raw-vs-escaped gap: its stat gate sizes base64 binaries, but text crossed unbounded. It now honours the same decoded-text limit the local branch already enforced. No wire change: GitDiffResult is untouched -- no third kind, no new field. Old clients see an error for one request instead of a dropped connection. diff_too_large joins the structured passthrough codes and lands on an existing error arm in both mobile consumers and the desktop remote path; file_too_large was already handled on both. Instruments the 1013 close, which nothing measured before, so the incidence this cap is meant to drive to zero is finally observable. `emitter` separates a producer size bug from a wedged link. Known regression: remote image previews between ~3.096 and ~3.146 MB now return file_too_large. They only intermittently worked before -- above ~3.0 MB they killed the socket -- so this trades intermittent connection loss for a consistent error. Test: 10281 passed in src/main/runtime + src/shared + src/main/git; mobile 3427 passed. Each of the six budget-enforcement sites is independently mutation-killed. Escaping fixtures cover newline-dense, control-char, CJK, lone-surrogate and base64 content against native JSON.stringify. tsc clean for node, web and cli; oxlint clean. Co-authored-by: Orca <help@stably.ai> * fix(runtime): harden remote reply transport budgets * test(runtime): cover desktop remote preview budgets * test(runtime): close telemetry review gaps * chore(shared): repoint budget imports after the shared/types barrel removal Upstream #14447 dropped the shared/types barrel; GitDiffResult now lives in git-diff-compare-types and GlobalSettings in global-settings-types. Co-authored-by: Orca <help@stably.ai> * fix(ssh): surface an over-cap preview read as file_too_large The stream reader aborts an over-cap read with StreamProtocolError, whose numeric code falls through mapRuntimeError to a generic runtime_error carrying the raw "Reported totalSize N exceeds client cap M" string. Neither preview client recognizes that: runtime-file-client.ts and mobile-file-preview-response.ts both key on file_too_large. It also made the two file_too_large guards directly below the read unreachable on the streaming path. Gives the cap its own error type so the caller can translate it, keeping the bandwidth saving the cap exists for. A genuine protocol fault still propagates unmasked. Found by the readiness review. Mutation-verified: removing the translation fails exactly the new test. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai> |
||
  Brennan BensonandBrennan Benson
|
d137bb93e1 |
fix(agent-status): stop start-less child stops from minting phantom working (#14375)
* fix(agent-status): stop start-less child stops from minting phantom working buildClaudeCachedLeadStatusPayload fell back to 'working' whenever the pane had no cached lead-turn state. That default is right for a spawn or a child tool call, but the same helper serves SubagentStop and TeammateIdle, which end work and prove the opposite. claudeLeadStateByPaneKey is in-memory only, so every app restart empties it. A Claude session that outlives the restart reports its next child event into an empty map and the pane latches 'working' with an empty roster -- no Stop ever clears it, and the 30-minute window only decays the sidebar dot, never the stored state. Fall back by the event's evidence: terminating child events resolve to 'done', which still gates up through resolveClaudePaneState when the roster or background work proves the pane is busy. * fix(agent-status): require evidence for child completion * fix(agent-status): publish matched teammate idle * fix(agent-status): preserve confirmed child work * fix(agent-status): retain live restored teammates * fix(agent-status): reap unconfirmed siblings after child drain * fix(agent-status): preserve unmatched restored children * fix(agent-status): wait for lead completion after child stop * fix(agent-status): persist restored child transitions --------- Co-authored-by: Brennan Benson <brennan@stably.ai> |
||
|
Neil
|
b6d36c26f4 |
fix(terminal): resolve Ctrl chords positionally on a non-Latin layout (#14469)
macOS maps a control chord by physical key regardless of layout: measured with UCKeyTranslate, physical A/U under Control produce U+0001/U+0015 on 2SetHangul, Russian and Greek exactly as on ABC, though unmodified those keys give ㅁ/ㅕ, ф/г, α/θ. A native terminal inherits this by passing the OS characters through. The browser does not expose that translation. xterm's legacy encoder works anyway because it reads keyCode, which Chromium reports from the physical key, but its kitty encoder derives the key number from `key` and only consults `code` when Shift or Option is held. Ctrl is not in that gate, so a pane with the kitty protocol negotiated reports CSI-u for U+3141 rather than 'a' and the chord does nothing. Ctrl+C escaped this only via its hand-written ETX bypass. Recover the byte from `code` when `key` is non-ASCII, which reproduces the OS control table. An ASCII `key` stays authoritative so a Dvorak remap is honoured. KeyC is excluded: the interrupt policy owns it, and off macOS that policy declines to a selection so the copy binding wins. Fixes #13331 |
||
|
Jinjing
|
b3efd48c7f |
test(e2e): stabilize terminal failure coverage (#14466)
* test(e2e): stabilize terminal failure coverage * docs(e2e): classify failed-run findings * rm report |