Skip to content

fix(cookbook): force remote commands through sh -c for non-POSIX login shells#5692

Closed
catagris wants to merge 1926 commits into
odysseus-dev:devfrom
catagris:fix/cookbook-remote-posix-shell
Closed

fix(cookbook): force remote commands through sh -c for non-POSIX login shells#5692
catagris wants to merge 1926 commits into
odysseus-dev:devfrom
catagris:fix/cookbook-remote-posix-shell

Conversation

@catagris

Copy link
Copy Markdown

Summary

sshd runs the remote command line through the target user's login shell, but Cookbook sends raw POSIX snippets — VAR=value assignments, export, if …; then, set -e — so on a server whose login shell is fish every remote operation dies at parse time: system-deps install fails with fish: Unsupported use of '=', the binary/prereq probes report tmux and every system package as missing regardless of reality, and download/serve launch + status/kill chains break (set -e is even a valid fish command that erases a variable instead of enabling errexit). This PR adds core.platform_compat.posix_remote_shell_cmd(), which wraps a snippet as sh -c '<snippet>' — a simple command every login shell (bash, zsh, fish, csh) parses identically — and applies it to every remote POSIX snippet: install-system-deps, the package/prereq/llama-server/GPU probes (whose old if/elif shell-fallback chain was itself POSIX-only syntax and died on fish before reaching any fallback), _remote_tmux_command/_remote_tmux_launch_command, the log-tail status commands in src/tools/cookbook.py and codex_routes.py, and the frontend command builders (_sshCmd, _shWrap for the tmux status/kill/alive polls — double-quoted so the local shell strips one layer and remote sh receives the snippet as one argument).

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #5689

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app and verified the change works end-to-end: deployed an image built from this branch against a real fish-login-shell CachyOS server — system-deps install runs (reaches its real sudo logic instead of dying at parse), the Dependencies system rows report true installed state, pip installs into a configured venv stream output and complete, and the tmux status/capture polls return data. Full test suite passes (4,679) including a new live-fish regression test that reproduces the unwrapped failure and proves the wrapped form executes.

How to Test

  1. Add a remote Linux server whose user's login shell is fish (chsh -s /usr/bin/fish on any test box) to Cookbook → Settings → Servers.
  2. Without this fix: Dependencies → install tmux fails with fish: Unsupported use of '='; system rows show everything as missing even when installed; downloads/serves to the host fail to launch or show no status. Quick shell-level repro: fish -c 'PATH="/x:$PATH"; command -v tmux' errors, while fish -c "$(python3 -c "from core.platform_compat import posix_remote_shell_cmd; print(posix_remote_shell_cmd('PATH=\"/x:\$PATH\"; command -v tmux'))")" succeeds.
  3. With this fix: the same flows work; tests/test_cookbook_remote_posix_shell.py covers the wrapper, the double-quoting layering, and the live fish round-trip (skipped when fish isn't installed).

Visual / UI changes — REQUIRED if you touched anything that renders

None — the static/js changes only alter generated shell-command strings (wrapping their remote side in sh -c); nothing that renders is touched.

🤖 Generated with Claude Code

pewdiepie-archdaemon and others added 30 commits June 27, 2026 21:10
Keep an unhealthy MemoryVectorStore instance available for health reporting instead of discarding it as disabled. This lets health checks report a degraded/down vector-store state while preserving focused regression coverage for initializer behavior.
RaresKeY and others added 27 commits July 11, 2026 15:06
…ls (#5420)

* fix: harden stabilization attachment and agent guards

* fix(uploads): preserve durable references during cleanup

* fix(uploads): close cleanup and compaction races
…(#4411) (#5160)

* docs: update static/js/MODULE_SUMMARY.md to reflect current ES6 frontend

Rewrite the stale module summary to match the current no-build,
ES6-module frontend architecture. Adds coverage of app.js orchestration,
the chat/SSE pipeline (chat.js, chatStream.js, chatRenderer.js,
streamingRenderer.js), new subsystems (research/, compare/, document
streaming, cookbook*, skills.js), and removes the obsolete <script> load
order assumptions.

* cleanup: remove dead MEMORY_DOC / memory_doc paths (closes #4411)

Removes the unused MEMORY_DOC constant and the matching DataConfig
memory_doc field / set_data_paths entry. No runtime code imports or
references these paths, so this is a no-behavior-change dead-code
cleanup under the storage-architecture tracker #4377.
* fix(db): restrict data/app.db to 0600

app.db holds bearer-token hashes, bcrypt password hashes, and encrypted
provider keys but was created under the default umask (0644 -> world-readable),
unlike .app_key/vault/integrations which are already 0600 via safe_chmod.

init_db() now chmods the SQLite file to 0600 right after create_all (POSIX
only; no-op on Windows, skipped for Postgres / in-memory). Unconditional and
idempotent, so it also re-locks already-deployed 0644 installs on next
startup. The transient rollback journal inherits 0600 from the parent file at
creation - no sidecar handling needed; -wal/-shm don't exist until WAL is
enabled (#4409 C4) and inherit the same mode then.

Satisfies Rule B, unblocking #4413 and the vault/integration secret moves.
Mirrors src/secret_storage.py:43-45.

Verified: security + DB-permission suites pass; 6 pre-existing visual_report
failures (missing markdown/nh3 deps) are unrelated.

Closes #4407

* fix(db): harden SQLite path parsing and re-lock sidecars

Address review feedback on #4420.

P2: derive the file to chmod from engine.url (SQLAlchemy's parsed URL)
via _sqlite_db_path(), instead of DATABASE_URL.replace("sqlite:///", "").
A driver-qualified URL (sqlite+pysqlite://) or one carrying query args
(?cache=shared) previously slipped past the prefix check / string slice
and left the DB world-readable; the parsed path resolves correctly and
drops the query.

P3: re-lock stale -wal/-shm/-journal sidecars to 0o600 at startup. The
main file is chmod'd first, so any sidecar SQLite creates afterward
inherits 0o600, but a -wal/-shm left world-readable by an older 0o644
install (once WAL was enabled) could still expose DB pages. Absent
sidecars are the normal case, not an error.

Tests: unit-test _sqlite_db_path across driver/query/memory/postgres URL
forms, and a subprocess test asserting stale 0o644 -wal/-shm are
re-locked on startup.

* fix(db): handle sqlite file URI app db permissions

* fix(db): close remaining SQLite permission bypasses

---------

Co-authored-by: Ethan <23321960+0xLeathery@users.noreply.github.com>
Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
chat_stream() references `_explicit_web_intent` in three places
(disabled-tools gating, global-disabled web allowance, and the
per-turn tool filter) but the assignment was dropped during a
branch merge. Every chat request raised

    NameError: name '_explicit_web_intent' is not defined

at routes/chat_routes.py, surfacing to the client as a bare
"Internal Server Error" before any LLM call was made — chat was
fully broken on dev and main.

Restore the original definition, computed from the already-derived
tool intent, immediately before its first use:

    _explicit_web_intent = bool(_tool_intent and _tool_intent.category == "web")

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 2d81770)
…mporter (#5261)

* Harden skill importer against SSRF: block private targets + revalidate redirects per hop

The skill importer validated only the initial URL with the lenient SSRF guard
(block_private=False) and then fetched with follow_redirects=True, so a 3xx to
an internal/metadata address (169.254.169.254, 127.0.0.1, RFC-1918) was still
connected to — inconsistent with the hardened services/search/content.py
:_get_public_url path.

Add a _get_checked() helper that follows redirects manually and re-runs the
SSRF guard with block_private=True on every hop, and route all three fetch
sites (skills.sh unwrap, _fetch_bytes, _list_github_dir) through it. GitHub's
own redirects and the final-host _assert_github_url checks are preserved.

Adds hermetic regression tests (IP-literal hosts, faked HTTP layer) and updates
the existing mock signature for the new block_private kwarg.

Defense-in-depth: the endpoint is admin-gated (require_admin) and admins are
trusted per THREAT_MODEL.md, so this is not a cross-boundary vulnerability.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: enforce follow_redirects=False invariant in mock client

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Add Arch-specific package installation and NVIDIA runtime configuration
to the Docker setup guide. Cover passthrough verification, the NVIDIA
Compose overlay, and the distinction between GPU passthrough and
CUDA-backed model serving

Refs #831
chore: sync upstream changes from odysseus-dev/odysseus
fix(docker): bump Docker CLI to a patched release
fix(docker): bump Docker CLI to a patched release
* feat(models): define capability schema and readers

* fix(models): harden Google catalog probing

Restrict native catalog probing to the Gemini host, keep provider keys out of request URLs, filter non-chat model resources, and preserve the manual refresh default in the built-in Google add flow.
…(#5474)

* security(url-safety): reject RFC 6598 shared address space in strict mode

Strict mode (block_private=True) is a full SSRF lockdown, but it only
rejected is_private and is_loopback targets. CPython does not classify RFC
6598 shared/CGNAT space (100.64.0.0/10) as is_private (it is "shared", not
"private"), so a public redirect into 100.64.0.1 passed the per-hop guard
and still issued the request to a potentially internal CGNAT service.

not is_global would also exclude it, but only on CPython 3.11.10+/3.12.4+/
3.13+; the CI matrix runs 3.11/3.12, so reject the range explicitly to stay
correct across patch levels and the 3.14 runtime image. Default local-first
mode is unchanged. Adds strict-mode coverage for shared, non-global, and
public targets.

* docs(url-safety): correct CGNAT is_global rationale in strict-mode comment

The prior comment claimed `not is_global` catches 100.64.0.0/10 only on
CPython 3.11.10+/3.12.4+/3.13+. That is inaccurate for CGNAT: is_global
is False for 100.64.0.1 on every supported version (verified 3.10-3.14).
The version-fragility applies to other ranges gh-113171 touched, not CGNAT.
The explicit range reject is still the right choice; restate the reason as
is_private not covering shared space, and not coupling strict mode to
is_global's broader, cross-version definition. No behavior change.
…… (#5491)

* fix(llm): enhance fallback logic to handle empty completions and improve metadata handling

* fix(llm): stream tool call deltas immediately
Slice 2f of the route-domain reorganization (#4082/#4071, per
specs/architecture-runtime-inventory.md §6.3). Moves note_routes.py into
routes/note/, leaving a backward-compat sys.modules shim at the old path.
Pure file reorganization, no behavior change.

The shim uses sys.modules replacement (same pattern as the merged gallery
#4903, research #4975, memory #5007, history #5090, and contacts #5227
slices) so that `import routes.note_routes`, `from routes.note_routes import
X`, `importlib.import_module(...)`, and the `import ... as note_routes` +
`monkeypatch.setattr(note_routes, "SessionLocal", ...)` pattern used by
test_note_reminder_fire_scope.py / test_notes_fail_closed_auth.py all
operate on the same module object the application uses.

The canonical module does NOT depend on the shim — routes/note/note_routes.py
imports only from core/, src/, and stdlib. The outbound email cross-domain
imports (routes.email_routes._get_email_config, routes.email_helpers.
_send_smtp_message) are function-local lazy imports that keep resolving
through the email module's own path (email is not yet migrated).

One source-introspection test site repointed to the new canonical path:
- test_model_helper_owner_scope.py (shared with history; history entry
  already repointed in #5090, note entry repointed here)

Adds tests/test_note_routes_shim.py to pin the sys.modules shim contract
(legacy and canonical paths resolve to the same module object; monkeypatch
via legacy alias reaches the canonical module).

Verified: compileall clean; full suite 4487 passed, 3 skipped.
…n shells

sshd runs the remote command line through the target user's login shell.
Cookbook sends raw POSIX snippets (VAR=value assignments, export,
if/then, set -e), so on a host whose login shell is fish every such
command dies at parse time: system-deps install fails with "fish:
Unsupported use of '='", the tmux/binary probes report tmux missing
regardless of reality, and download/serve launch + status/kill chains
break. `set -e` is even a valid fish command that does something
entirely different (erases a variable).

New core.platform_compat.posix_remote_shell_cmd() wraps a snippet as
`sh -c '<snippet>'` — a simple command every login shell (bash, zsh,
fish, csh) parses identically. Applied to:

- shell_routes: install-system-deps remote branch, venv-activated
  package probe, llama-server PATH probe
- cookbook_routes: _remote_binary_available, _remote_tmux_command,
  _remote_tmux_launch_command, remote setup scripts, termux detect;
  _run_gpu_shell's shell-fallback chain replaced with `sh -lc` (the
  chain itself was POSIX-only syntax, so it died on fish before
  reaching any of its fallbacks)
- src/tools/cookbook, codex_routes: if/then log-tail status commands
- frontend: _sshCmd (cookbook.js) and the tmux command/kill/alive
  builders (cookbookRunning.js) wrap the remote side in sh -c with two
  quoting layers (local shell strips one, remote sh gets the snippet
  as one argument); zombie-download probe reuses _sshCmd

Tests include a live fish round-trip that reproduces the reported
failure unwrapped and proves the wrapped form executes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… llama gpu, rebuild)

Follow-up to the posix_remote_shell_cmd sweep — three more _ssh_base_argv
sites sent raw POSIX snippets to the remote login shell:

- the system-prereq probe behind /api/cookbook/packages (PATH= + if/then),
  which made every system dependency row show "missing" on fish-shell
  hosts even after the package was installed
- the llama_cpp GPU-offload probe (venv activate prefix)
- the llama.cpp rebuild-engine command (full POSIX build script)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cookbook remote commands fail on servers whose login shell is not POSIX (fish): deps install, binary probes, tmux launch/status all break