The previous fold guarded each Delete/Move entry by running
get_write_denied_error on dirname(path). That coordinate is wrong both ways:
- runtime self-protection treats ANCESTORS of the running venv/interpreter
as protected, so a plain file directly in ~, ~/.hermes, the checkout root
or the uv python dir could no longer be deleted or moved ("'/Users/x' is a
protected system/credential file");
- credential-dir prefixes end in os.sep and match via startswith, so the
bare dir ~/.ssh never matched and a link directly inside ~/.ssh, ~/.aws,
~/.gnupg, ... was unlinked/renamed.
The existing classifier gains an entry=True mode (get_write_denied_error /
_classify_write_denial, and is_protected_path(follow=False)) that vets
realpath(parent)/basename — the entry, leaf not dereferenced — in addition
to the resolved target. A file inside a protected dir or prefix is denied;
a file merely beside the venv is allowed. delete_file and move_file make
one entry-mode call per entry instead of the duplicated path+dirname loop.
The new parametrized test covers both directions (plain Delete/Move next to
a monkeypatched runtime venv succeeds; a link in <home>/.ssh is refused with
link and target intact); all four cases fail on the previous fold head.
Delete and Move remove or rename the directory entry itself (a symlink,
not its target), but get_write_denied_error realpaths its argument, so it
only ever vetted the link's target. A link outside HERMES_WRITE_SAFE_ROOT
(or inside ~/.hermes/sessions) pointing at a file inside the root passed
the guard and the link was deleted/renamed outside the allowed area.
delete_file and move_file now also run the same guard on each entry's
parent directory, which realpaths to where the entry really lives.
The symlink test gains a safe-root case (red without this change), and
its Move cases always assert success, the rename, the link target and
files_modified instead of tolerating a refusing move primitive; expected
values are parameters rather than header introspection, and json is a
module-level import.
patch_tool rewrites every V4A header to the path _resolve_path_for_task
returns, and on a host backend that is Path.resolve(), which follows a
symlink in the last component. For Update/Add that is harmless: the shell
layer reads and writes the target through the link either way. Delete and
Move act on the directory entry, so "*** Delete File: config/local.yaml"
(a link to base.yaml) deleted base.yaml and left the link dangling, and
"*** Move File: current.txt -> previous.txt" renamed the link's target,
both reported as success.
Delete headers and both Move endpoints now resolve their parent directory
only (_resolve_entry_for_task), keeping the final component, so the link
is removed or renamed. The same paths are locked and reported in
files_modified. Update/Add headers are unchanged.
(cherry picked from commit 7b1fe43d30db6015155349b67ca814b340df0e18)
The rm of the cell result file runs after the runner has executed the cell.
A transport failure there propagated out of _run_remote_cell, so
_run_attached_cell evicted the kernel and re-raised, and the caller's
per-call fallback then ran the same code a second time.
Catch and debug-log that failure (a leftover cell_res_* is harmless because
seq is monotonic), so the atomic ship is the only remote call that can raise
inside the discard scope and the handler's "runs exactly once" comment holds.
The cell ship now fails closed (RuntimeError). _execute_remote catches it
and falls back to per-call execution, but the kernel stayed registered
with cell_seq bumped. The next call would then reuse a kernel whose
state silently missed this cell, with no state_lost flag. Discard (kill)
it the way the timeout path does, so the next call starts a fresh
kernel. The request never reached the runner (atomic publish), so the
fallback still runs the code exactly once.
Gate follow-ups on the remote lockdown:
- _execute_checked(env, cmd, what, **kw) in code_execution_rpc replaces
the three copy-pasted "execute, raise if returncode != 0" blocks
(per-call setup, kernel dir setup, file ship via
_remote_write(check=True)). env.execute always returns a dict, so the
isinstance/(r or {}) guards go; the error carries command output only,
never the payload.
- _ship_env_file_and_launch_prefix returned a half-built "( ... && "
that both callers had to close; a caller that dropped the ")" or
composed it differently would lose the load-bearing subshell. It now
takes the launch command, builds the shared env map (RPC dir, token,
PYTHONDONTWRITEBYTECODE, routed TZ) itself, and returns the complete
command; the kernel passes only HERMES_KERNEL_DIR/PYTHONPATH, which
drops its duplicated TZ block and lazy hermes_time import.
- _private_dirs_cmd(root, *subdirs): every caller spelled each path
twice for mkdir and chmod.
- _run_remote_cell publishes the cell request with one atomic
_remote_write instead of ship-to-.tmp then a separate unchecked mv,
saving a backend round-trip per cell.
_remote_write branched on getattr(env, "_stdin_mode", "pipe") and only
passed stdin_data on pipe backends, echoing base64 into argv elsewhere.
BaseEnvironment.execute already embeds stdin_data as a heredoc for
heredoc-mode backends (modal/daytona/vercel), and managed_modal forwards
it as stdinData; _write_to_sandbox already relies on that for every
backend. The branch duplicated base-class logic, and its defensive
getattr default meant a fake env with neither _stdin_mode nor a
stdin_data parameter raised TypeError on every RPC response write. The
poll loop swallowed the error, so no res_* file appeared and
test_code_execution_file_rpc hung forever (it passes on base).
Collapse to one path that always passes stdin_data, and teach the
file-RPC Shell fake to accept it and feed it as input. ScriptedEnv no
longer needs its _stdin_mode stub.
On shared remote backends the execute_code channel created kernel and
sandbox dirs under shared temp at the process umask (775 group-writable
under umask 002), wrote request/result files group-readable, and carried
HERMES_RPC_TOKEN on remote command lines where co-tenant users read argv
via ps for the whole run. A co-tenant could read tool arguments and
results, and on group-writable dirs forge RPC requests dispatched under
the user's approval context.
- All remote dirs are created owner-only (umask 077 + chmod 700, checked
fail-closed) and every Hermes file write is mode 600.
- The token travels in a sourced env file inside a subshell so the vars
never enter the backend's session-snapshot dump, and ships via stdin on
pipe-capable backends so it never enters argv at all.
- The RPC poll loop rejects non-int seq requests before dispatch instead
of replaying them every cycle.
- tool_result_storage gets the same owner-only treatment for archived
tool output.
(cherry picked from commit aef21731d7fb8a4e0a6ada4ff9889264df4a8893)
A dispatcher SIGKILLed between _call_spawn_fn and _set_worker_pid leaves a
live worker on a run with worker_pid NULL. release_stale_claims only extends
an expired claim for a recorded live pid, so on TTL expiry it reclaimed the
card and spawned a second worker beside the first: double billing, double
side effects, and a board showing one clean completed run (the first
worker's kanban_complete is refused as stale). Main CI hit it in
test_dispatcher_sigkill_mid_tick_never_destroys_or_duplicates_cards.
The worker now records its own pid on its run before the first model call
(adopt_worker_pid, worker_registered event, host-local claims only) and
exits without working the card when its run was already reclaimed. The
reclaim UPDATE also compares worker_pid so a registration landing between
the stale-claim SELECT and the UPDATE keeps the claim.
Repro: temporary sleep between spawn and pid record + kill 0.2 s after the
spawned event + slow first model reply -> 4/4 red on main with the CI
signature, 8/8 green here.
Fixes#121556
execute() can pass stdin_data="" (e.g. write_file of empty content). The
`is not None` guard then paid an upload (plus chmod on Daytona) and a
longer shell command just to feed zero bytes. Base heredoc mode skipped
empty stdin, and neither SDK exec attaches a stdin, so treating "" as no
stdin gives exactly the base command (checked: identical argv, no upload).
state["staged"] was set only after the upload returned. A kill() during
the upload therefore saw nothing to scrub, and exec_fn then hit the
cancelled gate and returned 130 without deleting. The staged file (which
can hold the sudo password line) stayed in the sandbox, whose filesystem
persists by default on both Daytona and Vercel.
Factor the scrub into a lock-held helper and call it from exec_fn's
cancelled branch as well as from cancel(). Also skip the upload entirely
when cancel already won before it started.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
Daytona and Vercel built the staged stdin path and the
`exec 0< f || exit $?; rm -f -- f || exit $?` prefix byte for byte the
same. That prefix is the security handoff (the shell takes ownership of the
payload and unlinks it before the user command runs), so it should live in
one place: two small BaseEnvironment helpers next to _embed_stdin_heredoc.
Upload and cancel lifecycle stay per-backend.
Also document the "payload" _stdin_mode; the old comment still claimed
Modal/Daytona use heredoc, which no built-in backend does now.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The pinned SDK (daytona 0.155.0) upload_file() accepts bytes, so the host
NamedTemporaryFile round-trip was redundant and briefly wrote the merged
stdin (which can start with the sudo password line) to the host disk.
delete_file() is delete_file(path, recursive=False); the extra
request_timeout kwarg raised TypeError inside contextlib.suppress, so the
pre-dispatch cancel scrub silently never ran and the staged payload stayed
in the sandbox /tmp.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The script already rm-s the staged file before running the user command,
so the success-path delete_file was a wasted round-trip, and post-cancel
deletes ran against a stopped sandbox. Only delete on kill() when the file
was uploaded but exec was never dispatched (checked under the env lock,
bounded request_timeout); exec_fn skips dispatch once cancelled.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
#122218 stopped the sandbox on ANY exec exception (killing background
processes on a transient SDK error, even for stdin-less commands) and
re-stopped/rm-ed on every cancel, failing the existing cancel test.
Track staged/dispatched under the env lock: kill() overwrites the staged
file only if it was uploaded but never dispatched, then stops once as
before. After dispatch the user shell opens and unlinks it itself, so no
extra command runs. exec_fn skips dispatch if cancel already won.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The transport publishes _resolved_identity and server_run resets it, but the attribute was
neither in __slots__ nor set in __init__, so a task that never reached the transport raised
AttributeError on read and the attribute silently lived in the mixins' __dict__. Declare it and
start it at None (not shareable until the transport publishes a digest). The getattr in
registration stays for test fakes that are not MCPServerTask instances.
discover_mcp_tools binds the owner secret scope only around the config load (#113746), so a
routed profile's reconciliation ran with no ambient scope. The adopter's stdio identity then
resolved unscoped, and with a source-tagged secret name get_secret raised UnscopedSecretError:
the stack's None sentinel kept that safe (the share was refused) but two profiles holding the
same value never shared the owner's child.
The omitted-name config load and the per-name identity resolution now run under this profile's
own secret scope (_owner_secret_scope), outside the registry lock.
Salvage resolution: the out-of-lock, once-per-name resolution, the None refuse sentinel and the
per-server refusal were already on the stack (_adopter_identity_digest / resolved_ids), so this
keeps that one implementation and takes the contributor's scope binding. The contributor's
unscoped-routed test is folded as an assertion into the kept multi-credential test (test budget);
their 'one unresolvable identity refuses only that share' test duplicates the kept 'boom' case and
is dropped, as are the test tweaks written against their _resolved_identity signature.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
(cherry picked from commit a8627071e7367cd544af77f921b4403a0b6c3e36)
_adopter_identity_digest returned "" for a config with no url/command and
for an unavailable live endpoint, but None for a resolver failure. Only
None is refused unconditionally by _same_server_route; "" is a comparable
string, so two unconnectable sides (an owner record of "" and an adopter's
"") compared equal and adopted. Return None on every can't-resolve branch
so there is a single refuse sentinel.
The non-owner reload test used an empty config and relied on that
""=="" match; give it a connectable URL config so the adopter resolves a
real identity.
Every multiplex discovery pass resolved an identity (secret-scope reads,
PATH lookup, live-endpoint probe) for each judged name, although the
digest is only read by a cross-profile `_same_server_route` comparison,
and that needs another profile's connection for the name. A profile's
first pass, and names it owns itself, paid for nothing. The first
registry-lock snapshot now also collects the names held under a foreign
scope and only those are resolved, still outside the lock; a foreign key
that appears after the snapshot has no digest and is refused, as before.
The same up-front loop let any resolver error other than
LiveEndpointUnavailable escape and abort discovery for the whole scope,
even for servers that share nothing. Such an error now refuses adoption
for that one server (None digest, fail-closed) and is logged once at
warning.
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
The module function `_resolved_identity(name, config)` (adopter
recomputation) shared its name with the attribute
`server._resolved_identity` (owner's published digest) and the
`resolved_identity` kwarg; a gateway test monkeypatched the function
while its fake set the attribute, which read as one thing. Renaming the
function to `_adopter_identity_digest` makes the owner/adopter split
visible at every call site.
Cross-profile adoption only works while the owner (transport) and the
adopter (`_resolved_identity`) hash byte-identical inputs, but each side
assembled the list itself: the stdio owner unpacked `_stdio_launch` and
re-listed `[command, safe_env, stdio_cwd]`, and both HTTP sides ran
`_http_endpoint` -> `_apply_identity_header` as separate copies. A new
launch field or header overlay added on one side would silently stop
every share (fail-closed, but no error).
`_connect_inputs(name, config)` now returns the list both sides digest
(stdio `[command, env, cwd]`, HTTP `[url, headers]`) plus the configured
header names the strict-redirect boundary needs, so `_run_stdio`,
`_run_http` and the adopter hash the same object by construction.
Co-authored-by: John Paul Soliva <soliva.johnpaul@icloud.com>
The stale-overlay loop re-derived each name's config by hand
(`servers` first, else the profile config) right next to `resolved_ids`,
which is keyed off `judged`. Two copies of one precedence rule means an
edit to either lets the static config and the resolved digest compared
in `_same_server_route` come from different sources. Read both from
`judged`.
_same_server_route recomputed the adopter's resolved identity per
candidate while _register_connected_into_current_scope held _core._lock:
a PATH lookup for stdio commands, secret-scope reads and the live-endpoint
AppResolver probe all ran under the global MCP registry lock, repeatedly
per live key. Resolve once per judged name before taking the lock and
pass it in via a resolved_identity kwarg (None refuses the share).
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Both are consumed by the HTTP transport but were in neither config_fingerprint nor
_connection_identity, so a profile whose config differs only in TLS verification or
redirect-header policy adopted another profile's live connection under that profile's policy.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
(cherry picked from commit 8b7175a2cf0e1935ea77805db9b99e4cba9fdcea)
The connecting loop hashed the resolved inputs, then the transport resolved them again; a runtime-file rotation between the two reads published endpoint A's digest alongside a session opened with B. The transport now resolves once per attempt (_http_endpoint / _stdio_launch), connects with that value and publishes its digest; the adopter recomputes through the same resolvers. The loop clears the digest per attempt.
(cherry picked from commit f543a4d3aadcdf7ec10093ebb99e14a740d43494)
The cross-profile identity digest covered config headers, identity_header,
secret-source env and cwd, but not two more per-profile resolutions the
transport performs: _run_http() swaps in a server_json live endpoint (URL and
bearer token) read from the active profile, and _run_stdio() resolves a bare
npx/npm/node through _resolve_stdio_command(), which can land under the
profile's own HERMES_HOME. Two profiles with those differences hashed equal
and could share one live connection.
_resolved_identity now resolves both exactly as the transport does. The new
test module imports the repository YAML module and passes explicit encodings
so it collects under scripts/run_tests.sh and clears the Windows-footgun gate.
(cherry picked from commit 637872d55bc81609657ed82139546709f6b99ccd)
_same_server_route judged "same credentials" from the static config alone, but a
connection is also opened with per-profile values the config never shows: a stdio
child's env carries every external secret-source value (Bitwarden, 1Password,
secrets.command) from the owner's secret scope, identity_header value_from: profile
resolves to the owner's profile name, and a stdio child's default cwd is the owner
session's runtime cwd. A multiplexed profile with a byte-identical config therefore
adopted the owner's live connection and its MCP calls ran as the owner.
The connecting task now records a SHA-256 of those resolved inputs on every connect
attempt, in its own scope; cross-profile adoption and the stale-overlay check
recompute it under the adopter's scope and refuse to share on a mismatch. Only the
hash is kept. Same-profile checks are unchanged.
(cherry picked from commit e59d0dfb9918db03482ccd4bf5a7599efc3cd72e)
The helper docstring restated the inline why-comments (exact probe string,
backend $HOME tilde fallback, which statuses prove absence). Keep only the
contract callers need: three return values and that anything not proven
absent is "unavailable" and must fail closed. Inline comments stay.
_probe_regular_file returns "bad_size" only after `[ -f ]` succeeded and
`wc -c` output was unparseable, so a regular file IS present on the
execution target. Treat it as "exists" (the precise existing-binary
refusal) instead of the generic "unavailable" retry message; both refuse,
but the retry hint is wrong for a file that is known to exist. Mirrors
how the other _probe_regular_file callers treat bad_size.
Idea and the original report/Docker reproduction come from #122663, the
first submitted fix for this bug.
Fixes#122662
Co-authored-by: liuzikaii <2319582736@qq.com>
_check_binary_document_write stat'ed the controller host only, so a binary
(.sqlite/.pdf/...) that existed solely in the task's execution target
(Docker/SSH/... namespace) was treated as a new file and destroyed by a
plain-text write_file/patch that even reported verified:true (#122662).
The existence decision now goes through one tri-state helper
(_target_regular_file_state: exists/absent/unavailable) that asks the LIVE
file-ops layer - the same backend the write executes on. Host-backed envs
keep today's Path.is_file semantics (OSError -> proceed); other backends are
probed via ShellFileOperations._probe_regular_file, whose missing/not_regular
answers prove absence while every other status fails closed with a retry
message. Locality comes from the live environment object
(_file_ops_uses_host_paths), never env_type strings or class-name hint tables
(VercelSandboxEnvironment is unclassified there). The PDF and generic binary
refusal messages are unchanged verbatim; opaque-document and SQLite-sidecar
unconditional refusals are untouched.
_stale_overwrite_blocker keeps its host-only probe on purpose: remote reads
never record a full_write_baseline (version stability requires host metadata),
so converting it would refuse every remote overwrite of a file the task fully
read. Tracked as follow-up.
(cherry picked from commit 8b5eb26d57d7ef7d1d975de0ef5e1e8abd9a513d)
Follow-up to the ssh path fix salvaged from #121686.
- Read the ssh anchor raw (session record, override, TERMINAL_CWD): the
shared workspace-root helper expands ~ on the Hermes host, so
TERMINAL_CWD='~/proj' still resolved into the container home.
- Resolve ~ to the remote home the SSH environment detects at connect,
bringing the environment up through the file tools' own creator
(_get_file_ops, same cwd and cache) when none is live. A failed bring-up
is remembered per container for 30s so one call's several resolutions
don't each retry; a live environment is always used first. SSHEnvironment
now records whether the home was detected, and a guessed /home/<user>
(echo $HOME failed) is not used. Results are
absolute and stable from the first call (read tracking and staleness
checks key on them), and '..' normalizes to the real target: relative
traversal like ../../../etc/x from ~ was refused on main and slipped past
the sensitive-path guard on the PR head.
- If the remote home cannot be detected, an ssh ~-path that climbs above ~
cannot be classified; the write guard refuses it.
- ~user passes through for the remote shell instead of becoming ~/~user.
- coerce_ssh_remote_cwd maps paths under the host subprocess home onto ~/,
except when that home is the OS user's real home.
- The outside-workspace warning compares in the remote namespace (it fired
on every correct relative write when the anchor was ~).
- The backend type is looked up once per resolution again (the PR head did three per local path).
The session cwd can already be the Hermes host's /opt/data/home. That
path is not on the SSH target, so the implicit cd exits 126 and the
command never runs.
(cherry picked from commit 4e99397bb4e18416b6a3e0b850d51604c0ad0d1d)
Relative writes and a bare ~ were expanded against the container
subprocess home and then sent to the SSH target, which does not have
that directory.
(cherry picked from commit f4a2549d8276762e53ed2d773dbd4de8c7e74b3b)
Drop the unreachable consumer-side _capped() wrapper (the pump already
stops before enqueueing past the cap), bound the pump queue and make
every put poll the stop event so an abandoned consumer can't wedge the
pump thread. Timeout message derives from _RECV_TIMEOUT_S; sample_rate
reuses DEFAULT_XAI_SAMPLE_RATE.
GeminiStreamer and the synchronous _generate_gemini_tts both sent the key
as a ?key= query param, so requests' HTTPError text (full prepared URL)
carried it into TTS logs on any 4xx/5xx — and text_to_speech_tool logs
unexpected exception text. Send it as the x-goog-api-key header in both
paths instead.
The unterminated-think flush half of the original PR is dropped: current
main already normalizes each clause before provider use (4aac89b429).
(cherry picked from commit 02387442c9a3830e1b4b7a727722472f2f0f76c1)
The old shape used extra_headers and a guessed message framing, and the
tests mocked the seam. Rewritten against the verified wire protocol:
voice/language/codec/sample_rate ride in the URL query string, bearer
auth header, text.delta + text.done out, base64 audio.delta envelopes in
until audio.done. Covered by a loopback WS test suite that pins the wire
format, early chunk delivery, and error-envelope raises.
The pump enforces the 16 MiB per-sentence byte budget before enqueueing
frames and closes the socket when it is exceeded: the consumer-side
_capped() runs only after q.get(), so without a pump-side check a fast
upstream piles decoded PCM into the queue ahead of the consumer. A
consumer that stops early flips a stop event so the pump closes instead
of filling a queue nobody drains. A lowered-cap loopback test verifies
the producer stops consuming upstream data.
(cherry picked from commit e6b5e29930fc026a759a1c205eb0fa97dd4a5688)
The #37589 uv/uvx fallback spelled its directory table (~/.local/bin,
/opt/homebrew/bin, /usr/local/bin) inline in _launcher_fallback, which the
managed-runtime ratchet flags as an unreviewed known_path_table outside
hermes_platform/. known_dirs is the module whose contract is 'every table in
Hermes lives here', so add uv_tool_dirs() there and compose it at the call
site. Behavior is unchanged: same four directories probed in the same order
(managed <home>/bin first, then uv's install order), bare uv/uvx still
resolves under a GUI-style PATH that lacks them.
macOS Desktop/launchd processes inherit the bare /usr/bin:/bin:/usr/sbin:/sbin
PATH, which carries none of uv's install locations, so an MCP server configured
as `command: uvx` fails with ENOENT at execvp from Desktop even though it works
from an interactive terminal. The stdio resolver already falls back to
well-known install dirs for bare npx/npm/node; extend the same treatment to
uv/uvx, probing managed <home>/bin, ~/.local/bin (uv's installer default),
/opt/homebrew/bin and /usr/local/bin (Homebrew AS/Intel).
Design salvaged from #37665, #67125 and #67178.
Co-authored-by: Morad37 <mohamed.origami@gmail.com>
Co-authored-by: Ignacio Rodriguez <ignacio@agenticolabs.io>
Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
After the locate click, type now refuses unless the located editable is
document.activeElement, so characters are not delivered as page input when
focus stayed on body. The keystroke loop checks an abort signal between
characters; preview.act cancel (timeout or interrupt) and a local Stop both
set it, so queued keystrokes stop. A printable press on body/html is refused
unless allow_shortcut is set.
A session asked to clean up older Pythons removed the uv-managed base
interpreter its own venv depended on; the next boot died with 'uv
trampoline failed to spawn Python child process' and no agent tool could
repair it, because the agent itself no longer started (#58748). Prior
uninstall detection (85ce25687e) only flagged package-manager commands.
Add agent/runtime_self_protection.py and wire it into both layers:
- The approval floor (_floor_block) now blocks shell commands that
delete the running interpreter, its own venv, the pyvenv.cfg base, or
the uv-managed install directory — rm/rmdir/rd/del/erase/Remove-Item
with any flags, find <root> -delete, and uv python uninstall of the
running version (including --all). The floor runs before yolo /
approvals.mode=off / cron approve mode, so no session setting can
bypass it.
- The file-safety write classifier denies write/patch/move/delete to the
same paths, so the file tools cannot overwrite the interpreter either.
Only the runtime the process itself boots from is protected; every other
venv and interpreter on the machine stays manageable.
Fixes#58748
[ -f ] and [ -e ] follow symlinks, so a dangling link probed as missing and
read_file_raw reported not_found: V4A Add followed the link and created its
target, Move replaced the link, both reporting success. The shell size
probe, the compound read probe and the native stat now classify any
symlink entry as not a regular file.
The fence drops backend output around the read, not output printed while
the transport runs: a BASH_ENV DEBUG hook firing for base64 alone puts
text inside the payload that still decodes (TERM is b"LDL"), and the edit
paths wrote it back. Emit wc -c in its own fenced segment and hand bytes to
a writer only when their length matches; the od fallback is checked the
same way.
Consolidates #120559 by @JoaoMarcos44 into this PR. Both PRs landed on the same
shape for #120514 — read_file_raw must be a byte-preserving mutation boundary,
framed so the backend's merged stdout is never decoded as payload — but the
sibling one step in FRONT of it was still unfenced here.
_sample_file_bytes whitespace-joined the whole reply before base64-decoding it,
so a backend that announces something on connect had that noise decoded into the
sample: "TERM" is four base64 characters and prefixes the sample with b"LDL".
That sample is the binary-admission gate read_file_raw consults, so noise there
decides whether a file is editable at all and what a refusal reports about its
bytes. Same class as the read this PR already fixed, one call earlier.
_fenced_read() is that framing extracted once — sentinel, payload segment, and
the body's own exit status in its own trailing segment — and the three byte
transports (sample, base64, od fallback) now share it instead of repeating the
split/parse/decode triplet, with _failed_read() for the diagnostic they all
surfaced by hand.
The mock double for the sample transport moves to the fenced reply shape the
other doubles in that file already build, so it stays honest about the fence.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
