Skip to content

feat(files-api): SSH-backed write for SaaS workspaces (fixes 500 docker not available) - #1702

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/files-api-saas-ssh-write
Apr 23, 2026
Merged

feat(files-api): SSH-backed write for SaaS workspaces (fixes 500 docker not available)#1702
HongmingWang-Rabbit merged 1 commit into
mainfrom
fix/files-api-saas-ssh-write

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Fix 500 failed to write file: docker not available on PUT /workspaces/:id/files/*path for SaaS workspaces (EC2 VMs, not Docker containers).

  • New writeFileViaEIC(ctx, instanceID, runtime, relPath, content) in template_files_eic.go
  • Uses the same EIC-endpoint + ephemeral-keypair flow as the Terminal tab (terminal.go)
  • Remote command: install -D -m 0644 /dev/stdin <abs path> — atomic write + creates missing parent dirs
  • Runtime → base-path map: hermes → ~ubuntu/.hermes, langgraph → /opt/configs, default /opt/configs
  • WriteFile + ReplaceFiles both detect workspaces.instance_id != '' and route to EIC
  • Local/self-hosted Docker path unchanged

Why this fixes the visible bug

Prod screenshot this morning:

API PUT /workspaces/a8af9d79-.../files/config.yaml: 500
{"error":"failed to write file: docker not available"}

SaaS tenants have no Docker to cp into — tenant platform + workspace both run as native processes on EC2. Code previously had no SaaS path at all.

Security

  • Absolute path is built from workspaceFilePathPrefix[runtime] (closed map) + filepath.Clean(relPath) — no user-controlled shell escape surface
  • validateRelPath rejects absolute paths and surviving .. segments
  • Remote command uses install, not shell redirect — no eval
  • shellQuote wraps the path as defence-in-depth
  • Ephemeral keypair lives in tmpdir for ≤ 30s, wiped on defer
  • EIC 60s key validity bounds the blast radius of any leak

Test plan

  • go test ./internal/handlers/ -run 'TestResolveWorkspaceFilePath|TestShellQuote' — 10 subtests pass
  • go build ./... clean
  • Manual: on hongmingwang, PUT /workspaces/:id/files/config.yaml → 200; verify ~ubuntu/.hermes/config.yaml on workspace EC2 shows new content
  • Manual: on local self-hosted (docker up), PUT should still route through docker and succeed

Follow-ups (not in this PR)

  • Reload hook after save (auto-restart hermes gateway via SSH)
  • Tunnel reuse in ReplaceFiles for bulk writes (currently one SSH session per file — ~3s per file)
  • Move the runtime → base-path map into the runtime manifest so new runtimes don't require handler changes

🤖 Generated with Claude Code

…er not available)

Symptom (prod, hongmingwang tenant, 2026-04-22):
  PUT /workspaces/:id/files/config.yaml → 500
  {"error":"failed to write file: docker not available"}

Root cause: WriteFile + ReplaceFiles always reached for the tenant's
Docker client, but SaaS workspaces run as EC2 VMs (no Docker on the
tenant to cp into). There was no SaaS code path, so Save/Save&Restart
in the Config tab silently 500'd for every SaaS user.

Fix: add writeFileViaEIC — same ephemeral-keypair + EIC-tunnel dance
that the Terminal tab already uses (terminal.go). Flow:

  1. ssh-keygen ephemeral ed25519 pair
  2. aws ec2-instance-connect send-ssh-public-key  (60s validity)
  3. aws ec2-instance-connect open-tunnel          (TLS → :22)
  4. ssh ... "install -D -m 0644 /dev/stdin <abs path>"
     install -D creates missing parent dirs atomically
  5. Kill tunnel + wipe keydir

Runtime → base-path map (new table workspaceFilePathPrefix):
  hermes     → /home/ubuntu/.hermes
  langgraph  → /opt/configs
  external   → /opt/configs
  unknown    → /opt/configs

Both WriteFile (single file) and ReplaceFiles (bulk) detect
`workspaces.instance_id != ''` and route to EIC instead of Docker.
Local/self-hosted Docker path is unchanged.

Security: the only variable piece in the remote ssh command is the
absolute path, which is built via map lookup + filepath.Clean so
traversal is blocked. shellQuote() wraps it as defence-in-depth.
validateRelPath rejects absolute paths and surviving `..` segments
up-front; tests assert traversal rejection.

Follow-ups tracked separately:
  - Reload hook after save (hermes gateway restart via SSH)
  - Per-tunnel batching for ReplaceFiles with many files
  - Runtime-specific base paths should be declared in the runtime
    manifest, not hardcoded in the handler

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@molecule-ai molecule-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: PR #1702 — feat(files-api): SSH-backed write for SaaS workspaces

APPROVE — Security + correctness verified

Security Analysis

1. Path Traversal (CWE-22 / F1085) — PROTECTED

  • resolveWorkspaceFilePath calls validateRelPath(relPath) BEFORE using relPath
  • validateRelPath (templates.go:65): checks filepath.IsAbs(clean) + strings.HasPrefix(clean, "..") — blocks both absolute and .. escape attempts
  • filepath.Join(base, filepath.Clean(relPath)) — Clean normalizes a/../b to b before Join, preventing mid-path .. bypass
  • workspaceFilePathPrefix bases are hardcoded constants — not user-controllable
  • Test coverage: TestResolveWorkspaceFilePath_RejectsTraversal confirms ../etc/shadow, /etc/shadow, ./../../etc, a/../../etc all rejected
  • shellQuote adds defence-in-depth: single-quote wrapping + backslash-escape, with TestShellQuote confirming correctness

2. SSH Command Injection (CWE-78) — PROTECTED

  • sshArgs is a []string array — passed to exec.CommandContext, NOT via shell eval
  • absPath is built from a map + Clean() — already traversal-blocked before it reaches the ssh args
  • install -D -m 0644 /dev/stdin — stdin redirect prevents any shell interpretation of content
  • StrictHostKeyChecking=no + UserKnownHostsFile=/dev/null — acceptable for EIC tunnel to localhost

3. Authorization Scope — CORRECT

  • WriteFile and ReplaceFiles routes are gated by WorkspaceAuth middleware (router.go:439-442)
  • WorkspaceAuth enforces: per-workspace token OR org-scoped API key OR ADMIN_TOKEN
  • instance_id is read from the workspaces DB row, controlled by the provisioner — not caller-supplied
  • SaaS branch only activates when instanceID != "" — Docker path unchanged for local containers

4. SaaS Mode vs Local/Dev — CORRECT

  • instanceID != "" -> EIC SSH path (SaaS/EC2 workspaces)
  • instanceID == "" -> Docker CopyToContainer path (local Docker containers)
  • Error on missing instance_id: "workspace has no instance_id — not a SaaS EC2 workspace" — clear and actionable

Architectural Quality

  • EIC helpers (sendSSHPublicKey, openTunnelCmd, sshCommandCmd, pickFreePort, waitForPort) are package-level vars, testable by stubbing — same pattern as terminal.go
  • Ephemeral keypair: ssh-keygen -t ed25519, stored in temp dir, defer os.RemoveAll(keyDir) on both success and failure paths
  • EIC key is only valid for 60s on the instance (AWS enforced) — no persistent key material
  • eicFileWriteTimeout = 30s bounds the whole dance; context.WithTimeout cascades to all subprocesses
  • region and osUser via env vars — consistent with terminal.go provisioning model

Minor Observations (not blockers)

  • ReplaceFiles partial failure: if EIC write fails for file N of M, earlier files are already written with no rollback. Follow-up to track. Not a blocker — HTTP 500 prevents the caller from assuming success.
  • No EIC integration test: only unit tests exist (resolveWorkspaceFilePath, RejectsTraversal, shellQuote). Integration test would require AWS credentials + EIC Endpoint. Pre-existing pattern in terminal_test.go — can follow as follow-up.
  • Runtime->base-path map: changing base paths without a migration orphans saved files. Acknowledged in code comments and TestResolveWorkspaceFilePath_KnownRuntimes. Not a regression.

Test Coverage

  • TestResolveWorkspaceFilePath_KnownRuntimes: 7 cases covering hermes/langgraph/external/default paths, case-insensitive lookup, empty/unknown runtimes
  • TestResolveWorkspaceFilePath_RejectsTraversal: 4 cases (absolute, leading .., multi-.., mid-path ..)
  • TestShellQuote: 3 cases including embedded single-quote edge case
  • All tests on a new template_files_eic_test.go file

Verdict

The SSH-backed write path is well-designed and security-conscious. Path traversal is blocked before the ssh command runs, injection is prevented by array-arg exec (not shell), authorization is correctly scoped to the workspace token, and the EIC ephemeral-key flow mirrors the already-reviewed terminal implementation. No changes requested.

@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit 7207133 into main Apr 23, 2026
9 of 10 checks passed
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
…#1702/#1730/#1731

Confirmed via gh CLI (GH_TOKEN restored): langchain-ai/langgraph PRs #6645, #7113, #7205
still OPEN as of 2026-04-23T17:38Z. A2A live-today positioning vs LangGraph in-progress
remains accurate. Logged PR #1731 (sweepPhantomBusy), PR #1730 (45-min gh-token refresh daemon
fixing 60-min 401 in long sessions), and PR #1702 (SSH-backed file writes for SaaS — P1
regression fix). Blog post for #1702 at docs/marketing/blog/2026-04-23-saas-file-api-fix.md.

Co-Authored-By: Claude PMM <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
PR #1702 (SSH-backed file writes for SaaS): blog post covers fix, compute
model detection, EIC-based remote write path. Ships same-day after merge.

PR #1686 (Tool Trace + Platform Instructions): full positioning brief —
buyer matrix, value props, competitive angle vs Langfuse/Helicone/OPA,
objection handlers, cannibalization assessment (LOW).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
…#1702/#1730/#1731

Confirmed via gh CLI (GH_TOKEN restored): langchain-ai/langgraph PRs #6645, #7113, #7205
still OPEN as of 2026-04-23T17:38Z. A2A live-today positioning vs LangGraph in-progress
remains accurate. Logged PR #1731 (sweepPhantomBusy), PR #1730 (45-min gh-token refresh daemon
fixing 60-min 401 in long sessions), and PR #1702 (SSH-backed file writes for SaaS — P1
regression fix). Blog post for #1702 at docs/marketing/blog/2026-04-23-saas-file-api-fix.md.

Co-Authored-By: Claude PMM <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 23, 2026
PR #1702 (SSH-backed file writes for SaaS): blog post covers fix, compute
model detection, EIC-based remote write path. Ships same-day after merge.

PR #1686 (Tool Trace + Platform Instructions): full positioning brief —
buyer matrix, value props, competitive angle vs Langfuse/Helicone/OPA,
objection handlers, cannibalization assessment (LOW).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
molecule-ai Bot added a commit that referenced this pull request Apr 23, 2026
…#1867)

* PMM: update ecosystem-watch — add LangGraph PR verification deferral note

- Add 2026-04-22 entry: GH API 401 for external repos, LangGraph PRs
  #6645/#7113/#7205 still VERIFY. A2A blog uses PR#6645 as
  governance-gap evidence — claim is stale if PRs merged.
- Update maintenance footer date to 2026-04-22

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* PMM: add Cloudflare Artifacts positioning brief

Source: PR #641, merged 2026-04-17.
Buyer: Platform engineers + enterprise security/compliance.
Headline: 'Give your agents a Git history — without touching a terminal.'
Objections covered: 'Why not GitHub?' + 'Cloudflare Artifacts is beta.'
Blocking: Social Media Brand launch thread.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* PMM: update EC2 SSH launch brief — social copy APPROVED, TTS audio file added as blocker

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* PMM: update ecosystem-watch — verify LangGraph PRs still OPEN, log PRs #1702/#1730/#1731

Confirmed via gh CLI (GH_TOKEN restored): langchain-ai/langgraph PRs #6645, #7113, #7205
still OPEN as of 2026-04-23T17:38Z. A2A live-today positioning vs LangGraph in-progress
remains accurate. Logged PR #1731 (sweepPhantomBusy), PR #1730 (45-min gh-token refresh daemon
fixing 60-min 401 in long sessions), and PR #1702 (SSH-backed file writes for SaaS — P1
regression fix). Blog post for #1702 at docs/marketing/blog/2026-04-23-saas-file-api-fix.md.

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

* docs(marketing): add PR #1702 release note + PR #1686 positioning brief

PR #1702 (SSH-backed file writes for SaaS): blog post covers fix, compute
model detection, EIC-based remote write path. Ships same-day after merge.

PR #1686 (Tool Trace + Platform Instructions): full positioning brief —
buyer matrix, value props, competitive angle vs Langfuse/Helicone/OPA,
objection handlers, cannibalization assessment (LOW).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(mmm): add Phase 34 positioning one-pager + messaging matrix

- phase34-positioning.md: one-pager with positioning statement,
  audience matrix, problem/solution, competitive differentiators,
  and proof points for press kit use
- phase34-messaging-matrix.md: 3 candidate taglines (production-grade,
  observability, aspirational) + full 4-feature messaging matrix
  (Partner API Keys, Tool Trace, Platform Instructions, SaaS Fed v2)
- SaaS Federation v2 flagged as content gap — no PM brief exists;
  community copy blocked pending PM confirmation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Molecule AI PMM <pmm@agents.moleculesai.app>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/files-api-saas-ssh-write branch April 24, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant