Repository navigation
fix(isolation, safety): keep CanvasTTY's private data out of agents' reach - #161
Open
BIackFIame wants to merge 37 commits into
Open
BIackFIame wants to merge 37 commits into
BIackFIame wants to merge 37 commits into
Conversation
If closing the Even G2 companion threw, quitting skipped terminalManager.shutdown(), so PTYs were not hung up and sessions were not saved. Shutdown now runs its steps in order through runShutdownSteps, which logs a failed step and continues.
…e off
A Codex or OpenCode subagent's final answer could belong to an earlier
turn: OpenCode reused its session id as the turn id and reported idle
before its asynchronous answer read finished, and recordAnswer accepted
any answer. OpenCode now names each turn, reports no idle before the
answer is read and drops a read a newer turn superseded; retry counts as
a turn start. A submitted input (not an approval answer, not an empty
Enter in an idle card) starts a new task, and only an answer of the turn
that answers it is kept. A signal of that turn which predates the input
still moves the status, so wait_for_agent ends instead of hanging, but
brings no answer. An answer reported as the agent exits still counts.
main's signal glue is now applyRuntimeSignal, shared with the tests.
With CanvasTTY status hooks off a launch gets no hooks, so no answer can
be captured. Whether a launch has them is decided when it is planned
(deferred plugin launches included). wait_for_agent and get_agent_result
then return answerUnavailable { reason: "status_hooks_off", message } so
the orchestrator reads the output instead.
Every write of person or agent input now goes through one method,
writeInput, which does this bookkeeping; input() and deliverInput call it,
and a later writer that holds input and replays it must call it too, so a
replayed Enter counts as a submitted prompt. It writes nothing into a
process the card no longer runs.
…UseFailure Once a turn id was active, the gateway dropped every non-start event with a new turn id. Each hook runs on its own connection and can time out, so when a turn's start hook was lost the whole turn, including its Stop, was ignored and the card kept the previous turn's status. The gateway now tracks whether a turn is open. After an idle, an unseen turn id opens the next turn; during an open turn, or for an id already seen, the event is still dropped as out of order. When a tool the person approved fails, the agent CLI runs PostToolUseFailure instead of PostToolUse. CanvasTTY did not register it, so the card stayed needs_approval until the next hook although the turn went on. It now maps to working.
node-pty reports a process killed by a signal as exitCode 0 plus the signal. The signal was dropped, so an agent killed with SIGKILL (OOM, kill -9) was recorded as "done, exit 0": wait_for_agent returned done, get_agent_result said done, and the saved session restored as exited. Record such an exit the way a shell does, as 128+signal (SIGKILL -> 137), mark the session failed, and put the signal name first in the failure details. Every reader of the exit (wait_for_agent, get_agent_result, the session store, the HOME failure summary) already derives its state from the exit code, so they now agree. Exits CanvasTTY causes itself (closing a card, quitting) are unaffected: those sessions are removed before the kill, and the exit handler ignores them.
A subagent spawned without launch options ran on its CLI's own default model and sign-in (for OpenCode the hosted OpenCode Zen model), even when its orchestrator ran on a model the person picked through a plugin's launch options. A plugin the spawn does not name now keeps the orchestrator's own values for a subagent of the same CLI; repeating the person's choice needs no delegable declaration, and then the normalized values must equal the orchestrator's exactly (missing keys come from the orchestrator, never from the plugin's defaults). A subagent of another CLI must name a plugin whose contribution set the orchestrator's model, and spawn_agent reports servedBy (CLI, requested model, inherited or explicit options). - Recognise every form a CLI takes for its model: --model and --model=, -m and -mX for OpenCode, Codex, Qwen, Kimi and Grok, and for Codex also --profile/-p and -c/--config setting model, model_provider or profile. - Refuse a second model (an explicit model next to a plugin that sets it, or two plugins that both set it; OpenCode crashes at start on two --model values) only for a subagent's launch. A person's own launches and restores start as before. - Inherit an empty choice (a plugin without fields) as chosen. Leave out an inherited plugin that was turned off since, unless it set the model (then refuse); refuse with a clear reason when its fields changed. - Reset modelSetBy at the start of each launch. - Describe inheritance, servedBy and the refusals in the orchestrator skill and docs. The core stays plugin-agnostic: it reads the plugin from the launch options and the contribution, never from a fixed plugin id.
OpenCode drops every key that arrives while its full-screen view starts (a few seconds). CanvasTTY typed spawn_agent's first prompt, send_to_agent and plugin sessions.send as soon as the PTY existed, so an OpenCode subagent sat at an empty prompt until the orchestrator gave up. deliverInput now waits, within the same bounded wait it already had for the launch, until OpenCode can take input: the terminal control sequence that shows its text cursor on the alternate screen once its prompt has focus, or a lifecycle hook from its plugin, counted only 2 s after the launch's first one (the plugin can report before the prompt takes keys). Only DEC private mode sequences are read; human-readable screen text is never parsed and no status changes. Other CLIs keep getting input when the PTY exists. - The wait is used on macOS and Linux. Windows' pseudo console may not pass those sequences, so there input is typed when the PTY exists, as before. - spawn_agent (checks, launch and readiness together) and send_to_agent wait at most 45 s, so the orchestrator gets the reason and the session id, not an MCP client timeout. Nothing is typed then. - A handoff waits up to 15 s for the CLI to take input instead of failing at once; a caller that does not wait is told the state. - Sends to one card are delivered in the order they were made, a queued send never reaches a later launch, and a send cancelled while it waits behind another settles at once and types nothing.
wait_for_agent accepted up to 600 seconds, but the MCP clients of several supported CLIs (OpenCode among them) end a tool call after about 60 seconds, so an orchestrator that asked for a long wait got a failed call instead of a result. A call now asks for at most 100 seconds (default 50). Unless the orchestrator's CLI is known to let a tool call run longer, a call returns after at most 50 seconds. A timeout carries the status and the masked output as progress plus a message to call again, and the tool description in both catalogs, the orchestrator skill and the docs say that a timeout means the subagent is still running.
Base protection is documented to refuse use of CanvasTTY's private data, but its hooks only saw shells and file writes: Claude Code's Read, Grep, Glob or LS, and the Qwen Code and OpenCode equivalents, could read provider-secrets.bin, the control token or another account's home without a check. While base protection is on at launch, the decision hook now also matches those read-only tools (Claude Code and Qwen Code matchers, OpenCode's guard through CANVASTTY_RUNTIME_DECISION_READS). With base protection off, a decision plugin's fail-closed hook still covers only shells and writes, so reads never wait on the gateway. They map to a read action that is checked only against the private data, with the same rules as a shell read: the agent's own account home stays its own, a search walks its folder, and nothing outside the private data is refused or asked about. Decision services are not asked about reads. OpenCode's grep without a path is sent as a search of ".", and glob's pattern is always sent. Qwen Code's read_many_files is refused when its paths and include globs together exceed 32, instead of checking only the first 32, and include globs are checked. A cut read-only call is refused like a cut shell call. Codex has no read-only file tool in its hooks; it reads through its shell, which is already checked. The docs now say base protection is best effort: it does not see paths built at run time or folder walks started above the app's data, so a project at the home folder needs agent isolation to keep the data private.
The private-data rule matched globs one component at a time, so `**`
stood for a single folder and `cat ~/Library/**/provider-secrets.bin`
was allowed; brace expansion was not seen at all, so
`cat <userData>/provider-secrets.{bin,}` was allowed too.
- `**` now matches zero or more folders. A glob that only passes by the
app's folder on its way to other names (`~/**/*.ts`) is not counted.
- Unquoted braces in a shell word, and braces in a Glob tool pattern,
are expanded (at most 64 words) before matching.
- On macOS and Windows a glob matches private names in any letter case.
Base protection also had limits that let the rest of a call through
unchecked. A call now is refused ("uncheckable") instead when:
- a path field is not a string, or is longer than 4096 characters after
folding `./` and `a/..` padding;
- read_many_files names more than 32 paths and include globs, or an
apply_patch touches more than 256 files or a too-long path;
- the private-path budget (now 1024 checks), the brace-expansion limit,
the path candidates in one word or heredoc (256), or the nesting of
shells and substitutions runs out;
- checking a shell, file-edit or read call fails with an error. Before,
an exception in the analyzer let the call through; other tools still
get no opinion.
An apply_patch whose command is longer than 4096 characters is read in
full; before, it was treated as touching no file.
bubblewrap bound the shared gateway socket folders writable into every isolated agent, so an agent could replace a gateway socket and receive other agents' requests. It also took a socket folder from dirname() of every CANVASTTY_*_ADDRESS in the launch environment, which holds the person's variables and plugin contributions: $HOME/foo.sock bound all of HOME after the tmpfs masks, and an address inside plugin data reopened every account home. The control grant folder was read from the same environment. Socket folders are now bound read-only (connect() needs no write access to the folder) and come only from the sockets CanvasTTY's own gateways prepared for the launch; the control grant only from the control environment the host gave an orchestrator. A socket that must not be read is masked with /dev/null, and a gateway socket folder that holds a hidden path is refused. On macOS the seatbelt Unix-socket allow list is built from the same gateway sockets.
… them With a project that contains userData (the home folder), an orchestrator could start a subagent in <userData>/plugin-data/<plugin>/homes. The isolation layer reopened any project at least two folders deep in plugin data, so that subagent could read and write every account's sign-in. A subagent's folder may no longer be inside CanvasTTY's private data, unless the project itself is a folder inside that same plugin's data (an environment's worktree). The layer reopens a project inside plugin data only when the host placed the launch there: a folder an environment prepared inside its own plugin's data, or the folder the person chose for the card the launch descends from. Anything else is refused with the reason. Also: a gateway socket folder that contains a writable folder is refused instead of mounted over it; a trailing separator in a CLI home variable names the same account home; the docs list which variables count as an account home, in English, Russian and Chinese.
…ct that contains it When an isolated agent's project contains CanvasTTY's userData folder (a project at the home folder), the whole project was writable, so the app's settings.json and the plugins folder were writable to the agent; only the private parts were hidden. The isolation paths now carry readOnlyFolders (userData when a writable folder contains it) and fixedFolders (its parents inside that folder). Seatbelt denies writes in userData after the project allow, re-allows the writable folders inside it (a selected account home) and denies renaming or replacing the parents. bubblewrap binds each parent onto itself (a mount point cannot be renamed), mounts userData read-only over the project and binds the writable folders inside it again. A project located inside userData is unaffected.
…worktrees Inside the isolation layer only a repository's hooks, info/attributes and an existing .git/config were protected. .git/commondir (where git reads the shared config and hooks from), .git/config.worktree and the linked worktrees' records in .git/worktrees stayed writable, and the path-based rules could be sidestepped by renaming .git, .git/hooks or .git/info away and back. Seatbelt now denies writes to commondir, config.worktree and worktrees/ in any repository of the project, existing or not, and denies renaming or removing .git, .git/hooks and .git/info. For an existing repository the paths list config.worktree and worktrees/ (Linux mounts them read-only, with neutral placeholders when missing) and an existing commondir; on Linux the project's .git is bound onto itself so it cannot be renamed. Objects, refs, the index, logs and HEAD stay writable.
Base protection treated anything inside the working folder (and the agent's config folder) as the agent's own before it checked CanvasTTY's private data. With the project at the home folder, or at any folder that holds the app's userData, an agent's shell command or file tool could read or overwrite provider-secrets.bin, the control token, other account homes and the gateway records. A project in a temporary folder likewise reopened the gateways' socket folders. A folder is now the agent's own only below the private data: a private path or socket folder inside the project or config folder stays private. A folder inside a private path (the account home the agent was launched with) is still its own, and its neighbours are not. A recursive walk of a folder in the app's data that holds private paths is refused wherever the project is. The per-card Claude Code draft folders (<userData>/claude-draft) are listed as private data too; nothing creates them yet.
… immutable A database without -wal/-shm was always opened with immutable=1, which skips locking and hot-journal checks, so a rollback-journal database could be read while Hermes was writing it. Use the immutable read only when the header says WAL (bytes 18 and 19 are 2) and no side files exist; open other databases with mode=ro and retry SQLITE_BUSY a bounded number of times before skipping the poll.
…edential rules Held values were masked leftmost-first and the scan jumped past each match, so a second held value overlapping the first showed its suffix. Masking held values before the generic rules could also erase a label such as password or Authorization that a rule needed, leaving the credential visible, and each call allocated text-sized typed arrays. Find held values, JSON values and credential shapes as source ranges on the original text and mask the union of overlapping ranges; search held values with indexOf/KMP streams and a ring buffer instead of text-sized arrays; count entropy characters without regex match arrays. The empty-registry path is unchanged.
…ere read The Files reader and the usage history reader bind an opened descriptor to its path by dev/ino. As numbers, 64-bit file ids (NTFS, some network file systems) lose precision, so two different files could compare equal. Compare bigint stats instead. The history reader also compares the pre-open lstat with the descriptor, so a file swapped between the symlink check and the open is refused rather than read and then quarantined. When invalid records are dropped, the original is preserved from the bytes read through the checked handle (exclusive create, mode 0600) instead of a second copy by path.
wait_for_agent, get_agent_result and observe_agent returned the raw PTY
tail, which for a full-screen TUI such as OpenCode is cursor moves,
colours and spinner redraws, often starting mid-sequence. They now render
the masked scrollback window (the last 65,536 characters) through the
existing headless xterm at the session's size and return the visible
text, masked again and capped as before (observe_agent within its
maxChars). A cut window starts at its first escape or line break, so the
rest of a cut sequence ("18;9H") is never printed as text.
exitLines were the regex-stripped last 16 KiB, so a TUI that redraws only
the cells that changed read as its redraws joined together, and a line
drawn early and never redrawn (the model's status line) was lost. They
are now rendered by the same headless terminal from the same window,
masked before and after, and the three tools return the same exitLines.
An isolated agent could read every plugin's private data folder (<userData>/plugin-data), including every CLI home and token an accounts plugin keeps there, not only the account it runs on. plugin-data is now private app data for the isolation layer. The one folder reopened is the CLI home a chosen launch contributor started the agent with: the provider's own home variable set to an exact absolute folder strictly inside that contributor's host-assigned data folder. The host derives it from the contribution; a plugin cannot name a grant. AgentIsolation checks it again when the layer is built (real folders, no symbolic links from plugin-data down) and refuses the launch otherwise, giving the reason once. A project inside plugin data is reopened as that one folder. The Grok/Kimi status hooks already take their account home from the same contributor record (dataDir) and the same accountHome field; that field now comes from this one rule (contributedAccountHome), and the hooks still use it only when it is the GROK_HOME / KIMI_CODE_HOME the CLI reads, so the home is checked once. The path helpers are shared with accountHomes.ts.
Claude Code keeps its Bash tool output and external-editor files in
${CLAUDE_CODE_TMPDIR:-/tmp}/claude-<uid> and ignores TMPDIR. The isolation
layer does not make /tmp/claude-<uid> writable, so every shell call of an
isolated Claude failed with "Operation not permitted".
Set CLAUDE_CODE_TMPDIR to the launch's private temp folder for Claude on
both layers (seatbelt and bubblewrap). The shared /tmp/claude-<uid> is not
granted: all of the person's Claude sessions use it, and an isolated agent
could otherwise rewrite another session's task output.
…card's own account home An accounts plugin keeps every account's sign-in in CanvasTTY's plugin data folder. Base protection's private list had account-homes but not plugin-data, so an agent outside the isolation layer (a person-launched card, or one with isolation off) could read another account's tokens. Add plugin-data to the private list. The account home the card's launch plugin selected (the home the launch pipeline already validates for the isolation layer) is recorded with the card and counts as one of its own config folders, as CLAUDE_CONFIG_DIR does, but only while the CLI is still started with it. Other accounts' homes, other plugins' data and the listing stay refused; a project CanvasTTY placed inside plugin data keeps its own files. plugin-data is not added as a name marker, since such a project names that folder in its own paths.
The Kimi provider runs Kimi Code CLI, which keeps its configuration and OAuth sign-in in KIMI_CODE_HOME (default ~/.kimi-code); the older kimi-cli uses KIMI_SHARE_DIR (default ~/.kimi). The isolation layer only knew KIMI_HOME, which neither CLI reads. So: - a Kimi launch whose home was moved with KIMI_CODE_HOME (an account home a launch plugin hands it, e.g. one sign-in of an account pool) did not get that folder as its own writable state, and - other CLIs inside the layer could read ~/.kimi-code, where Kimi Code's refresh token lives. Kimi's own folders are now KIMI_CODE_HOME (or ~/.kimi-code), KIMI_SHARE_DIR (or ~/.kimi), KIMI_HOME when set, and the usual named folders; all three variables move Kimi's home. Nothing else changes.
…atus hooks and tools Grok Build reads hooks only from GROK_HOME/hooks and Kimi Code only from KIMI_CODE_HOME/config.toml; Kimi also reads its MCP servers (mcp.json) and permission rules there. CanvasTTY wrote its lifecycle hook overlays and Kimi's browser/agents configuration into the default homes only, so a card a launch contributor started in its own account home (an account pool) ran without working/idle/needs_approval status and without the canvastty_browser / canvastty_agents tools. When a launch contributor sets GROK_HOME / KIMI_CODE_HOME to an account home below its own data folder (<userData>/plugin-data/<pluginId>, from the host's record), the launch pipeline reports it as the launch's account home and the same overlays are written there instead, shared by the cards in that home and removed when the last one ends. A variable pointing anywhere else, including another plugin's data, leaves the launch as before. Inside plugin data every folder from plugin-data down and every file CanvasTTY writes must be real (no symbolic link), or the launch is refused. The overlays keep the existing lock, journal, backup and marker rules, so the person's own settings are restored byte for byte. The checks live in accountHomes.ts and are shared by both overlays. Claude Code, Codex, Qwen Code and OpenCode take their hooks from the launch's arguments or environment and already work in any home.
…ts orchestrator An OpenCode subagent on a launch contributor's model keeps bash on "ask" (a launch contributor's other model keeps bash asking, like accept-edits elsewhere). When no decision service decides, OpenCode's own prompt waits in the subagent's card for the person. That part already works: the card turns needs_approval, Needs attention lists it, and wait_for_agent returns at once with the prompt in output. Two things stood in the way: - send_to_agent still typed into that card. Any prompt text ends with Enter, which answers OpenCode's prompt, so the orchestrator (on the same third-party model) could approve the command for the person. It is now refused while the subagent is needs_approval, with a reason that tells the orchestrator to ask the person and wait. - wait_for_agent returned the same pending prompt again on every call, so an orchestrator could only poll or give up. It now returns a prompt once; a later wait on the same prompt (the card has not changed since) waits for the person's answer and then for the subagent's turn, so the orchestrator carries on in the same turn once the person has answered. A new prompt is reported at once again. Unchanged: bash stays "ask" for a third-party model in every card; a decision service's deny still fails the call before OpenCode asks and its allow (from a plugin the person let allow) still answers OpenCode's prompt once; no verdict is never turned into an allow; person cards and OpenCode on its own model are untouched; no new UI. The test runs CanvasTTY's real OpenCode plugin, runtime gateway, decision hooks, TerminalManager and orchestration tools: allow answers the prompt, deny fails the call, no verdict reaches Needs attention and the orchestrator, send_to_agent writes nothing, and the person's answer resumes the subagent and the orchestrator's wait.
…s a subagent's approval prompt
Asks are answered by the person, or by a decision service the person
allowed, never by the orchestrator. The canvastty_agents tools already
refuse send_to_agent while a subagent is needs_approval. The control
endpoint did not: on an Orchestrator session's own connection, choose
answered Codex's approval prompt ("1. Yes, proceed") and dismiss answered
it with Escape ("No, and tell Codex what to do differently").
On an orchestrator's connection, choose, dismiss and send are now refused
with NEEDS_PERSON (not retryable, nothing is written) while the worker
is needs_approval, the status each CLI's own approval signal sets. For
Codex's choose and dismiss the menu is also checked: Codex can draw its
approval prompt before its PermissionRequest hook's report arrives, and
choose acts on that exact screen. codexApprovalPrompt matches Codex's
command, edits, permissions, network and MCP approval prompts, not hooks
review, folder trust, model or update menus.
Unchanged: menus that are not approvals stay the orchestrator's; status,
screen and result still read; interrupt still stops its own turn (the
asked call does not run); the person's own app-wide connection, the
card's input and decision services are untouched.
Tests run the real TerminalManager, AgentControlService, gateway and
control CLI request path with an orchestrator's session grant: approval
prompts refused before and after the hook report, non-approval menus
chosen and dismissed, the person's connection still choosing, and
send_to_agent refused at every CLI's approval signal and delivered once
the person answered.
…e UI scale
Some UI chrome kept fixed pixel sizes, so Settings -> Appearance -> UI
scale (0.85-1.25) did not reach it:
- The terminal card's options menu (Don't restore this window, plugin
card actions) had a 12px font and pixel paddings while the card header
around it scaled.
- The canvas shortcut-hints panel looked the same at 0.85 and 1.25.
- In Settings -> Plugins the plugin description scaled with the settings
panel, but the permission list, the contribution rows ("App on
canvas"), the plugin title, author, version, buttons, module and
native-hook lists kept fixed 9-17px sizes, so at 1.25 one card mixed
large and small text.
The menu and the hints panel now take their font size from
calc(Npx * var(--ui-scale, 1)) and size their paddings, gaps, radii and
minimum widths in em; the hints icon is sized in em too. The plugin
settings font sizes use the same calc. At scale 1.0 every size is the
same as before. UI_CONTRACT names the menu and the hints panel among the
chrome that follows --ui-scale.
The title bar is left as is: its height is shared with the native
window chrome (macOS traffic-light position, the startup page) and with
every viewport-bound overlay through --titlebar-height.
…like the reference
The Russian canvas hints and Settings still showed English: the
wheel/pinch hint, the hard-coded "Shift + drag" key cap in the hints
panel and the shortcut reference, the canvas navigation override
description ("wheel, pinch и drag ... Drag колёсиком") and the
Settings > Canvas wheel capture strings and Off/On/Key labels.
- Translate them, reusing the existing on/off keys and adding
shortcutShiftDrag and canvasWheelCaptureKey; the hints use one word for
the drag gesture ("перетаскивание"). The group-selection label no
longer repeats its key cap (ru/en). A test checks that these ru strings
hold no Latin words besides key names.
- The wheel/pinch and navigation override rows in Settings > Keyboard
shortcuts wrap instead of cutting their longer Russian labels off.
- The hints panel showed raw bindings ("Alt+ArrowUp") while the shortcut
reference showed "Alt+↑". Both use one formatter now. It maps key by
key, so on macOS "Meta+Comma" reads "Command+," instead of the
reference's ",nd+Comma" (the Comma key was replaced inside "Command").
The canvas wheel and navigation settings' text (only Settings shows it)
moves from the startup string table to canvasNavigationText.ts, next to
Settings, so it loads with the Settings chunk; renderer-bundle-splitting
checks it stays out of the startup bundle.
When GitHub returned a token but the /user request hit the request timeout, the timeout's AbortError was taken for a cancellation: the poll stopped silently, the flow was cleared and the status stayed "pending", so Cancel did nothing. Only this flow's own abort now counts as a cancellation; a timed-out profile request ends the sign-in as failed.
…or body
A transient 429/5xx from the token endpoint with a JSON body such as
{"error":"temporarily_unavailable"} skipped the transient-status check and
ended the sign-in as an unknown provider error after one answer. These
statuses are now retried with backoff whatever the body says; slow_down
keeps its own handling.
browser_click on an engine without layout returns when element.click() answers; the fake engine reports the navigation it started afterwards. Until Page.frameNavigated arrives the tab still shows the old document as ready, so h.ready() could return before the revision advanced and the test failed under load. h.ready() now takes the revision to wait past.
"terminal window descriptors persist atomically without scrollback or environment" saves a descriptor whose cwd is process.cwd() and then checks the whole file against /buffer|environment|capability|token/. Run from a checkout whose path contains one of those words (for example a worktree named after an environments feature), the cwd alone matched and the test failed on every run. The check now leaves the descriptor's own cwd value out; everything else in the file is checked as before.
…h files Closing a card started the run cleanup and the removal of runs/<id> at the same time, and a launch still being prepared (plugin prepare, environment wrap) could write run files after the folder was gone. A throw from the environment wrap also skipped the planned and contributed cleanup. Contributed launches are now tracked. Closing waits for pending launches, then the serialized exit/restart cleanups, then removes the run folders each pipeline created for this card object, and the card folder only when it is empty, so a card brought back under the same id keeps its new run. An unadopted launch releases its planned spawn and contribution in a finally block, and a launch whose card was closed or restarted in the same tick never asks its plugins. Run folders are removed with retries for files Windows still holds open.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This was referenced Oct 22, 2026
Contributor
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BIackFIame
force-pushed
the
pr/security-hardening-v2
branch
from
October 22, 2026 14:43
9d32d4f to
92690d1
Compare
Contributor
Author
|
Self-review while the bot review fails: fixed two bypasses in their commits. On macOS the project folder itself can no longer be renamed away and back to carry |
Contributor
Author
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
… delegated cards Agent cards inherited CanvasTTY's whole environment, so every agent, every subagent and every plugin-started card, isolated or not, received provider API keys, tokens and organization-level admin keys (OPENAI_ADMIN_KEY, ANTHROPIC_ADMIN_KEY) exported where the app was started. Now an agent card never inherits admin credentials (*_ADMIN_*KEY, *_ADMIN_*TOKEN) or CanvasTTY's own (CANVASTTY_GITHUB_TOKEN and any CANVASTTY_*TOKEN/KEY/SECRET/PASSWORD); the person's own top-level agent cards otherwise keep their environment, so CLIs signed in by API key keep working. Subagents and every card a plugin starts, a plain shell card included since the plugin drives it, inherit only an allowlist (path, home, user, shell, locale, terminal, temporary and XDG folders, Windows system folders, proxy and certificate variables, the CLIs' home folders); what they need beyond that comes explicitly through a launch contribution's secretEnv. The launch builders (agent runtime hooks, browser and orchestration bridge, OpenCode inline config) now read the card's filtered environment instead of the app's, so they no longer hand the person's OPENCODE_CONFIG_CONTENT to a delegated agent. A plain shell card the person starts keeps everything but CanvasTTY's own credentials. CANVASTTY_AGENT_PASS_ENV in the app's environment lists variables to keep anyway.
…'s shapes
OpenCode's decision bridge sent write, edit and multiedit all as toolName
"edit" with {file_path, content}. A decision service could not tell a
whole-file write from a partial edit, and an edit's replaced text was
dropped, so it could not see what the edit removes.
The bridge now uses Claude Code's names and shapes, which hooks and
decision services already read:
- write: Write {file_path, content}
- edit: Edit {file_path, old_string, new_string, replace_all}
- multiedit: MultiEdit {file_path, edits: [{old_string, new_string,
replace_all}]}
- bash and apply_patch are unchanged.
Base protection reads file_path for Write, Edit and MultiEdit as before.
A test goes through DecisionHooks: the service gets kind "edit", the
path and both texts, and a MultiEdit outside the project is still
denied. The docs (en/ru/zh-CN, plugin-api.d.ts) list the shapes
OpenCode's tools arrive in.
BIackFIame
force-pushed
the
pr/security-hardening-v2
branch
from
October 22, 2026 19:50
92690d1 to
b79de17
Compare
This was referenced Oct 22, 2026
The HOME limit row showed the used share as a bare "9%" with a bar and a reset countdown; "used" was only in the hover/screen-reader text. Codex's own status line shows "% left", so 9 % used read as 9 % left. The row now says "9% used · 91% left" (ru «9% использовано · осталось 91%»), and the hover/screen-reader line "9% used · 91% left" («использовано 9% · осталось 91%»). Usage history's latest observation says the same instead of a bare percent. The normalizers were right: Codex app-server usedPercent and Claude utilization are the used share; a test pins both with fixture payloads.
…a way to the setting A plugin page whose service could not run because the person had not trusted its native code (or had its module turned off) only got "Plugin service is not running.". The host now refuses those requests with a fixed text; frames show it localized (ru/en), and the host notice carries an Open settings button that opens Settings -> Agents -> Extension native code (or Settings -> Plugins for a module). A trusted service that crashed or failed keeps the generic text, now followed by its last error.
…led outcomes
A started GitHub device sign-in could not be cancelled, retried poll
failures until the code expired, and ended silently when GitHub answered
access_denied or expired_token. A sign-in that could not start showed
the raw IPC error ("Error invoking remote method 'github-auth:start':
...") at the top of the Plugins page, in English in the Russian UI.
- GithubAuthService.cancelDeviceFlow() is public and exposed through the
existing github-auth IPC (main-renderer only) and preload.
- status() reports deviceFlowState: idle, pending, denied, expired,
failed or cancelled.
- access_denied, expired_token, unknown provider errors and non-transient
HTTP errors stop polling with that outcome; OAuth error bodies on HTTP
400 are read as well.
- Network errors, timeouts, HTTP 408/429/5xx and unreadable bodies back
off and retry, but five failures in a row end the flow as failed.
- slow_down keeps adding five seconds to every later poll.
- The host clock ends the flow as expired even if GitHub keeps answering
authorization_pending.
- Settings shows a Cancel button next to the pending code and a localized
status line (en, ru) for the outcome, using existing classes: denied,
expired and failed use the Settings error style, cancelled is a plain
status line. A start or cancel failure shows a localized line ("not
configured" for a build without an OAuth client); the details go to
the log.
The new sign-in lines live in PluginSettingsSection (the lazily loaded
Settings chunk), not in the startup string table; renderer-bundle-splitting
checks they stay out of the startup bundle.
BIackFIame
force-pushed
the
pr/security-hardening-v2
branch
from
October 25, 2026 07:41
b79de17 to
4802c94
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This replaces #148 (reworked on top of current main after #158 and alongside #159; the old PR is closed with a link here).
Branch
pr/security-hardening-v2(head4802c945, 13 commits), stacked onpr/orchestration-reliability-v2(the PR that replaces #147), which is stacked on #159. No UI.Merge order: #159, then the orchestration reliability PR (#160), then this PR, then the subagent worktrees PR (#162). Until they are merged, this PR also shows their 24 commits.
Already in v1.8.2-test.2, and what this PR adds
All 13 commits of #148 shipped in the prerelease v1.8.2-test.2 ("isolation/private-data hardening from PR #148"). That build was cut from 1.8.1 on a hotfix line, so none of it is on
main. This PR brings the same 13 fixes tomainand makes them fit #159:One account-home rule: fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 fix 2 (status hooks and tools for Grok and Kimi in a contributed home) and this PR's first commit both recognised the CLI home that a launch contributor hands a launch. Each had its own rule,
dataDirrecord andaccountHomefield. Now there is one of each:LaunchContributor.dataDirand the fieldPreparedLaunch.accountHomecome from fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159;contributedAccountHome, which covers any CLI's own home variable;contributedHookHomeis removed;GROK_HOME/KIMI_CODE_HOMEthat the CLI reads (fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159'saccountHookHomecheck inTerminalManager, unchanged).The path helpers (
segmentsBelow,withoutTrailingSeparator) are shared fromaccountHomes.ts, and the trailing-separator handling moves into the first commit. fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159's test of the removed function now checks the same cases through the combined rule.ProviderRuntimeLaunch.preparetakesdecisionReadsbeforeaccountHome, and fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 fix 13'senvironmentafter them.Kimi's homes: fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 fix 1 makes
KIMI_CODE_HOMEandKIMI_SHARE_DIRKimi's own homes. The list of variables that count as an account home (docs in en/ru/zh-CN, code comment) now names them too.OpenCode decisions: fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 fix 3 sends OpenCode's
write/edit/multieditin Claude Code's shapes. This PR's read-only tools (read,grep,glob,list) are added next to them. The old assertion thatmultieditarrives as a singleeditis dropped.Why
When you closed #135 you asked us to drop its UI and keep only the isolation and enforcement hardening for behaviour
mainalready has, as PRs without UI. This PR has that hardening, plus four small fixes from the same audit of existing code (redaction, file-id checks, Hermes reads). Every fix closes a way an agent could read or change data that CanvasTTY keeps private: other accounts' sign-ins, the control token,provider-secrets.bin, gateway sockets and Git config.What changes for users
Agent isolation (sandbox-exec on macOS, bubblewrap on Linux):
CANVASTTY_*_ADDRESSvariable. Now only CanvasTTY's own sockets are bound, read-only.settings.json, plugins).commondir,config.worktree,.git/worktrees) are write-protected..git,.git/hooksand.git/infocannot be renamed away.${CLAUDE_CODE_TMPDIR:-/tmp}/claude-<uid>and ignoresTMPDIR. The layer never made that folder writable, so onmainevery shell call of an isolated Claude fails with "Operation not permitted". An isolated Claude now getsCLAUDE_CODE_TMPDIRset to the launch's own private temp folder, on both seatbelt and bubblewrap. The shared/tmp/claude-<uid>is deliberately not granted: all of the person's Claude sessions use it, so an agent could rewrite another session's task output.Base protection (without isolation):
claude-draft, which macOS: editing keys for Claude Code cards (newline, select-all, copy/cut/paste) and UTF-8 locale #145 adds under userData), although nothing creates them yet.**and{a,b}globs are matched correctly.plugin-datais now on the private list. An accounts plugin keeps every account's sign-in there, so an agent outside the layer (Manual, or isolation off) could read other accounts' tokens. The card's own account home counts as its own config folder, likeCLAUDE_CONFIG_DIR, for as long as the CLI is still started with it. This is the sameaccountHomethe launch pipeline already validates for the layer and for fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159's hooks. A project CanvasTTY placed inside plugin data keeps its own files.Other fixes:
immutable.Commits
f1591798fix(isolation): hide plugin data from isolated agents9a5adde7fix(isolation): bind only CanvasTTY's own gateway sockets, read-only2cbcccbdfix(isolation): open plugin data projects only where CanvasTTY placed themb50dc0a2fix(isolation): keep CanvasTTY's data folder read-only inside a project that contains it9a3594a8fix(isolation): write-protect the git files that redirect config and worktreesd7d78877fix(safety): keep CanvasTTY-private data private under broad projectseaf06f85fix(safety): refuse read-only file tools on CanvasTTY-private data8bb279e8fix(safety): match ** and brace globs, refuse what cannot be checked20313b40fix(redaction): mask overlapping secrets whole and keep labels for credential rules261beb66fix(files): compare file ids as bigints and preserve the bytes that were read3329e23dfix(usage): read rollback-journal Hermes databases with locks, not as immutable387a24d4fix(isolation): give isolated Claude Code its own temp folder4802c945fix(safety): keep plugin data private in base protection, except the card's own account homeThe messages, authors and dates are the same as in #148. Commit 1's message adds a paragraph on the shared account-home rule.
Commits 1–3 belong together: commit 1 alone is not safe on Linux without commit 2. Commits 6–11 are independent of 1–5 and of each other. Commit 12 is independent. Commit 13 builds on 1 (account home) and 6 (private list).
Verification
macOS: this exact tree (
4802c945), with a fake HOME and a short TMPDIR. Results:build:helpers --host, typecheck, build andaudit:secretspass. The full suite gives 2455 / 2451 / 0 fail / 4 skipped. The startup bundle is 1,691,536 bytes, the same as fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159. The seatbelt tests ran under realsandbox-exec.Targeted, after each conflict resolution:
agent-isolation,account-home-isolation,agent-runtime-account-homes(fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159's 14 account-home tests, end to end fromLaunchPipelinethroughTerminalManagerto both bridges),decision-hooks,base-protection-app-private,launch-contributors,agent-runtime-provider-launchandopencode-subagent-ask: all pass.Combined:
main+ fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 + fix(orchestration): reliable subagent launches, prompts, waits, answers and exits #160 + fix(isolation, safety): keep CanvasTTY's private data out of agents' reach #161 + feat: a worktree per subagent, a combined change review and rollback points #162 + the auto-sort PR (feat(canvas): add Auto-sort to the empty-canvas Tools submenu #163) merge without conflicts. That tree passes typecheck, build andaudit:secrets. Its full suite gives 2636 / 2630 / 0 fail / 6 skipped.Linux (Ubuntu 24.04 VM, bubblewrap 0.9, normal user, no sudo, earlier heads):
AgentIsolationandbwrap: hidden plugin data, read-only sockets, a refused unplaced project, a read-only data folder and protected Git files all behaved as described;CLAUDE_CODE_TMPDIR= its bound private temp, and writes to a shared/tmp/claude-<uid>fail with "Read-only file system";mainthere.This was not repeated on this rebase. The code those checks cover is unchanged apart from the shared account-home rule.
Screenshots: none needed (no UI).
Not verified: Windows. A live Claude Code session (Bash tool, Ctrl+G) inside the layer. Hermes was not run live. The bigint precision case cannot be produced on APFS, so it is covered by reasoning.
Risks
envor args loses access to it under isolation ({launchFiles}is the supported way).KIMI_SHARE_DIRandKIMI_CODE_HOMEinside its data, the last one counts as the card's account home. The hooks go there only when it is theKIMI_CODE_HOMEone. With fix: fifteen bug fixes in isolation, orchestration, UI, plugins, GitHub sign-in and tests #159 alone, onlyKIMI_CODE_HOMEwas ever considered.git worktree addinside the layer fails for repositories in the project.Depends on
The orchestration reliability PR (#160) and, through it, #159. Both share
TerminalManager's launch path with this PR, and the account-home record and field now come from #159.