Skip to content

chore: extract 3 small fixes from closed #1664 - #1735

Merged
HongmingWang-Rabbit merged 1 commit into
mainfrom
chore/extract-1664-small-fixes
Apr 23, 2026
Merged

chore: extract 3 small fixes from closed #1664#1735
HongmingWang-Rabbit merged 1 commit into
mainfrom
chore/extract-1664-small-fixes

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Three small, non-overlapping fixes extracted from closed PR #1664. Bundled because each fix is tiny, they touch independent files, and they share the same provenance (all unblocks or stability tweaks the original PR carried alongside bigger changes that won't ship).

Fix 1 — canvas ContextMenu.tsx: React error #185 (infinite snapshot loop)

ContextMenu previously derived the "does this node have children?" flag via a useMemo over a new array returned by (s) => s.nodes. The useMemo was stable across renders, but the selector itself returned a fresh reference every time an unrelated field in the canvas store changed, which Zustand's useSyncExternalStore saw as "snapshot changed" and scheduled a re-render -> another selector call -> another fresh array -> loop -> React error #185 on some prod sessions.

The new selector hashes down to a boolean (s.nodes.some(...)). Booleans compare stably with Object.is, so the snapshot is steady. The delete handler still needs the actual child nodes to prompt the user, so it reads them once via useCanvasStore.getState().nodes.filter(...) at click time, off the render path.

Fix 2 — workspace-server a2a_proxy SSRF bypass for local-docker

resolveAgentURL rewrites http://127.0.0.1:<port> to http://ws-<id>:8000 when the platform itself runs in Docker (Docker-bridge hostname). The rewritten URL then resolves to 172.18.0.x, which the SSRF guard blocks as a private IP — stranding A2A in local-docker dev.

The fix adds isInternalDockerCall and skips the SSRF guard for that path. Two narrow cases trigger the bypass:

  • The 127.0.0.1 -> InternalURL rewrite (new URL came from our own provisioner, trusted).
  • URLs already starting with http://ws- under platformInDocker.

SaaS safety: the ws-* branch is gated on !saasMode(). In SaaS the workspace registry is remote and an attacker-controlled registration could claim a ws-* hostname that resolves to a sensitive VPC IP; leaving the SSRF guard on there is correct. Local-docker is a trusted single-host setup where ws-* comes from our own compose/bridge network and the relaxation is safe.

Fix 3 — workspace-server Dockerfile: dynamic Docker socket GID + gosu/su-exec entrypoint

The platform binary needs the host docker socket to provision workspaces, but the socket's group varies between hosts (Colima ~= 101, Linux ~= 999, etc.). Hard-coding a GID breaks on the other platform.

The new entrypoint.sh:

  1. Reads the docker.sock group GID with stat.
  2. Creates a matching docker group inside the container and adds platform to it.
  3. Falls back to the root group only if the socket is owned by GID 0.
  4. exec su-exec platform /platform "$@" — drops privileges before running the binary.

Non-root runtime is preserved. Only the entrypoint runs as root long enough to adjust group membership.

Test plan

  • cd workspace-server && go build ./... clean
  • cd workspace-server && go test ./internal/handlers/ -count=1 -run "TestProxyA2A|TestResolveAgentURL|TestIsSafeURL|TestIsPrivateOrMetadataIP" — 14 failures, all pre-existing on main (DNS-lookup + sqlmock regex drift, unrelated to this PR; same count and same names before/after the change)
  • npx tsc --noEmit on canvas — no new errors in ContextMenu.tsx
  • Manual: local-docker dev still resolves A2A to ws- without SSRF rejection
  • Manual: platform container on Colima can docker ps via mounted socket
  • Manual: canvas no longer throws React fix(security): gate GET /approvals/pending behind AdminAuth (#180) #185 with mixed parent/child workspaces

🤖 Generated with Claude Code

…bypass + workspace-server Dockerfile GID entrypoint

Three small, non-overlapping fixes extracted from closed PR #1664:

1. canvas/src/components/ContextMenu.tsx — Replace the useMemo-over-nodes
   pattern with a hashed-boolean selector (s.nodes.some(...)) so Zustand's
   useSyncExternalStore snapshot comparison is stable. Resolves React
   error #185 (infinite render loop). Moves the child-node list derivation
   into the delete handler via getState() so the render path no longer
   allocates a fresh array.

2. workspace-server/internal/handlers/a2a_proxy.go — Allow the
   Docker-bridge hostname path (ws-<id>:8000) to skip the SSRF guard in
   local-docker mode. Gated on !saasMode() so SaaS deployments keep the
   full private-IP blocklist (a remote workspace registration can't claim
   a ws-* hostname and reach a sensitive VPC IP).

3. workspace-server/Dockerfile — Add entrypoint.sh that discovers the
   docker.sock GID at boot and adds the platform user to that group, then
   exec's su-exec to drop privileges. Lets the platform container reach
   the host docker socket without running as root.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread workspace/a2a_executor.py
if tool_trace and hasattr(msg, "metadata"):
try:
msg.metadata = {"tool_trace": tool_trace}
except (AttributeError, TypeError):
@HongmingWang-Rabbit
HongmingWang-Rabbit changed the base branch from staging to main April 23, 2026 03:02
@HongmingWang-Rabbit
HongmingWang-Rabbit merged commit 0d820bd into main Apr 23, 2026
3 checks passed
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the chore/extract-1664-small-fixes branch April 24, 2026 00:08
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…ace (#1737)

CTO-bypass merge 2026-05-24: all 5 CI sub-jobs verified success; umbrella stale due to status-propagation race; compensating success status posted. Persona acks in place.
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