Skip to content

chore: remove unused legacy assets and add open-source housekeeping - #1

Merged
PekingSpades merged 1 commit into
mainfrom
dev
May 15, 2026
Merged

PekingSpades merged 1 commit into
mainfrom
dev

Conversation

@zRzRzRzRzRzRzR

@zRzRzRzRzRzRzR zRzRzRzRzRzRzR commented May 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Cleanup pass before open-source release. Removes assets that are no longer
referenced by code, updates CI to track the current bundle manifest, and
adds the basic GitHub housekeeping files an open-source repo needs.

Removed (unused)

  • relay/cli-anything-wave1.json — legacy bundle manifest, replaced by
    relay/managed-command-providers.json. CI cache-key references in
    relay-ci.yml and relay-gui-build.yml are updated to the new manifest
    in this same PR.
  • .setup/plugins/amap/*.html (15 files) — unreferenced.
  • .setup/plugins/aminer/*.json (~26 files) — unreferenced.
  • .video/.seedance/v*/prompt*.txt (3 files) — unreferenced; the seedance
    skill at .agents/skills/seedance2-skill/SKILL.md is methodology, not a
    consumer of these prompt files.
  • docs/relay-auto-skills.md — internal note. Code mentions of
    relay-auto-skills resolve to ./relay-auto-skills.js or log labels;
    nothing reads the markdown.

Added

  • .github/ISSUE_TEMPLATE/bug_report.yaml — bilingual bug report template.
  • .github/ISSUE_TEMPLATE/feature-request.yaml — bilingual feature request template.
  • .gitignore: .idea for JetBrains users.

Test plan

  • CI green (cache key still valid via relay/managed-command-providers.json).
  • Issue templates render correctly on github.com.

@PekingSpades PekingSpades changed the title clear useless code chore: remove unused legacy assets and add open-source housekeeping May 15, 2026
@PekingSpades PekingSpades self-assigned this May 15, 2026
@PekingSpades
PekingSpades merged commit 32e3f5d into main May 15, 2026
10 of 20 checks passed
@zRzRzRzRzRzRzR
zRzRzRzRzRzRzR deleted the dev branch May 17, 2026 11:13
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ed IP

Review follow-ups zai-org#1 and zai-org#2:

zai-org#1 — downloadToBuffer (the generic entry used by saveFromUrl, and thus by MCP
image results, Zhipu image/parse URLs, files/ingest) had NO SSRF check and read
the whole body unbounded (arrayBuffer). It now delegates to
downloadToBufferWithLimit with a 50 MiB default cap, so every caller gets the
SSRF host check + streaming size cap.

zai-org#2 — assertPublicHost was pre-flight only: fetch re-resolved the hostname at
connect time, leaving a DNS-rebinding window. Add ssrfSafeDispatcher() — an
undici Agent whose connect-time lookup re-validates EVERY resolved address and
refuses non-public ones, so the address that passes the check is the one the
socket connects to. downloadToBufferWithLimit now fetches through it (plain
Agent when allowPrivateHosts is set, e.g. tests/internal). undici declared as a
direct api dep.

Tests: +2 (generic entry enforces SSRF; hostname→loopback blocked at connect);
storage 10 + ssrf 8 green.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
Review follow-up zai-org#1: my previous lock-loss fix did a bare `return`, which
bypassed the catch-block cleanup — leaving the session with a dangling
`running` turn and attached pending wakeups until external recovery swept them.

Throw a typed SessionLockLostError instead, so the existing catch runs:
markTurnWakeupsDropped + updateTurnStatus(turn, 'cancelled'). Add a dedicated
catch branch that, for lock-loss, cleans up the turn and shuts down MCP tools
but does NOT touch session status or requeue — the new lock owner now drives the
session. The outer finally's releaseLock is token-fenced (safe no-op on a lock
another worker holds) and decr(actorSessions) still balances our startup incr.

api builds clean; full api suite 1302/1302 green.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…se user msgs)

Review follow-up zai-org#1, two real defects in the prior lock-loss handling:

1. WRONG ROUTING: shouldAbortTurn returning lockLost makes actorThink throw a
   generic TurnInterruptedError, so the catch matched the INTERRUPT branch (not
   the lock-lost branch) and mutated session status / requeued as if we still
   owned the session — conflicting with "the new owner drives it". Also the
   lockLost flag was block-scoped inside the try, invisible to the catch.
   Fix: hoist lockLost to handler scope and branch on the FLAG (not the error
   type) in the catch.

2. LOST WAKEUPS: on lock loss the attached wakeups were marked "dropped", but
   the new owner only reads "pending" wakeups — so user-triggered wakeups
   vanished and the session went idle. Fix: add restoreTurnWakeupsToPending()
   (attached -> pending, clears turn_id/attached_at) and call it on lock loss;
   then requeue if any wakeups are pending so a worker re-claims the session
   (the Redis lock serializes who actually runs). No terminal session-status
   write — that's the new owner's call.

Tests: 2 new DB-backed wakeup-lifecycle tests (restore re-pends; drop stays
dropped). Full api suite 1294/1294 green.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ross side-effects

Two more lock-correctness defects from review:

zai-org#1 STRANDED WAKEUPS ON ACQUIRE-SKIP: session-thinking jobs are enqueued with no
BullMQ attempts, so a job that finds the session lock held just returns
"session locked" = done, no retry. Combined with the lock-loss restore path
(restore wakeups -> pending -> requeue), the requeued job could hit the new
owner's lock and skip, while that owner had already read 0 pending and gone
idle -> wakeups stranded with no driver. Fix: on acquire-skip, if there are
pending wakeups, re-enqueue a DELAYED "think" job (2s) so it re-claims once the
current owner releases. Eventually-consistent: it keeps retrying until a worker
holds the lock and drains the pending wakeups.

zai-org#2 RENEW DIDN'T COVER SIDE-EFFECTS: the fenced lock-renew interval was cleared
right after actorThink, but executeActorActions + message persistence +
turn/wakeup status + runtime/audit/event emits all run AFTER that, until the
outer finally releases. A side-effect phase longer than the 120s TTL could let
another worker concurrently take over. Fix: hoist the interval and clear it in
the OUTER finally instead, so the fenced renew covers the whole critical
section (model + all side-effects) until release.

api builds clean; full api suite 1296/1296 green.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ueue error

Review follow-ups zai-org#1 and zai-org#2:

zai-org#1 SIDE-EFFECTS WROTE AFTER LOCK LOSS: lockLost was checked once right after
actorThink, but executeActorActions, message persistence, turn/wakeup status,
audit/event, and the terminal idle/queued writes all ran afterward without
re-checking — so a lock lost DURING the side-effect phase still let the old
worker complete writes, racing the new owner. Add assertStillHoldLock() and
call it before executeActorActions, before message persistence, before
markTurnWakeupsProcessed/updateTurnStatus(completed), and before the terminal
status write. Each throws SessionLockLostError → catch restores wakeups +
requeues. (The renew now spans the whole critical section, so loss mid-side-
effect is unlikely; these checks bound the residual double-write window.)

zai-org#2 DISHONEST retryScheduled: the acquire-skip delayed re-enqueue swallowed a
queue.add() failure yet still returned retryScheduled:true — monitoring would
see "scheduled" while the wakeups were actually stranded. Let the add() throw
(the job then fails visibly / is retried) since that re-enqueue is the only
driver for the pending wakeups.

api builds clean; full api suite 1298/1298 green. Renew-returns-false-on-
takeover (the mechanism that sets lockLost) is covered by lock.test FENCING.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…nger hangs

Four review follow-ups on the lock-loss recovery:

zai-org#1 — replay flag set too early: sideEffectsStarted flipped true before
executeActorActions REGARDLESS of result.actions, so a pure-reasoning turn (no
actions, no message — executeActorActions is a no-op, persistence kind:"none")
was treated as replay-unsafe and its input wakeups were DROPPED on a lock loss.
Rename to replayUnsafeStarted and set it true only when a non-idempotent write
actually happens: result.actions.length > 0, or messagePersistence.kind !==
"none".

zai-org#2 — late independent wakeups stranded: the replay-unsafe branch skipped the
pending-wakeup check, so a message that arrived mid-turn (never attached to
this turn, and enqueueSessionWakeup won't enqueue a job while running) had no
driver. Now BOTH branches re-check getPendingWakeupCount after the restore/drop
and requeue if any remain. This never replays the current turn: in the unsafe
case its own wakeups were dropped (not pending), so only genuinely-unprocessed
input drives the re-run.

zai-org#3 — concurrency test hung (exit 124): it materialized the ioredis singleton
but never closed it. Add an `after()` hook calling shutdownRedisConnections().
Verified it now exits 0 even WITHOUT --test-force-exit.

zai-org#4 — test didn't cover the real decision: extract decideLockLossRecovery()
(pure: replayUnsafe → restore|drop, requeue iff pending remains) and route the
handler through it, then drive the concurrency test through that same policy.
Added the two regressions above as explicit cases (pure-reasoning replay-safe;
replay-unsafe-but-late-pending requeues).

api typecheck clean; full api suite 1370 pass / 0 fail / 25 skipped.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…s, auth-origin plumbing

High zai-org#1 (native email login/register session): the custom synapse_session cookie
name broke @better-auth/expo's cookie-jar (it only persists cookies matching its
prefix or *session_token). Revert to BA's DEFAULT cookie name
(better-auth.session_token) — no business code reads the cookie by name anymore.
getSessionBearerToken() now matches by endsWith('session_token') (handles the
__Secure- prefix). session-provider also prefers signIn/signUp data.token, with
the cookie-jar value as fallback.

High zai-org#2 (native Feishu OAuth return): refreshSession() now falls back to the
expo cookie-jar token (getSessionBearerToken) when SESSION_TOKEN_KEY is empty
and persists it — so a deep-link return from the OAuth browser flow establishes
the session.

Medium zai-org#3 (web device polling): pollDeviceToken() unwraps the RFC 8628 HTTP-400
error body (authorization_pending/slow_down/access_denied/expired_token) from
the thrown ApiError.details and returns { error } instead of throwing, so the QR
panel's normal-wait and terminal branches work.

Low zai-org#4 (mobile auth-origin plumbing): EXPO_PUBLIC_AUTH_ORIGIN wired through
setup.sh + docker-compose mobile-web build args + Dockerfile.mobile-web
(optional; defaults to API origin for same-origin deploys).

Verified: server emits better-auth.session_token + set-auth-token + body token;
simulated expo cookie-jar capture + bearer resolution succeed; api 1384/0/25;
web typecheck 0; integration 20/20; mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…evice-poll error unwrap

Medium zai-org#1: setup.sh only wrote AUTH_TRUSTED_ORIGINS in the new-.env template, so
an existing .env (pre-feature or customized) never gained $APP_URL + synapse://,
leaving the server's trustedOrigins empty -> @better-auth/expo can't return the
session on a native Feishu OAuth deep-link callback. Added ensure_csv_env_contains
(merge, not overwrite — preserves operator-added origins, no dup) and call it for
AUTH_TRUSTED_ORIGINS in the upsert region. Verified the helper across missing/
merge/noop cases.

Low zai-org#2: pollDeviceToken() only unwraps HTTP-400 responses whose body.error is a
known RFC8628 device error (authorization_pending|slow_down|expired_token|
access_denied|invalid_grant); 500/502/proxy/other errors now re-throw so the QR
panel surfaces a real failure instead of masking it as 'expired'.

web typecheck 0; setup.sh syntax OK.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…er label, plugin stableKey fixture)

- ref.test.ts: cast ToolDefinition via 'unknown' (TS2352) so tsc -p
  shared/tsconfig.json passes — this is what mobile's pretypecheck runs;
  the cast was a pre-existing dev failure, now fixed. (zai-org#1)
- from-generate-text.ts: an unknown provider-executed tool with no query now
  derives its display label from toolName instead of the generic '网络搜索';
  only true web_search-without-query keeps that label. (zai-org#6)
- ref.test.ts: plugin stableKey fixture → production format
  'plugin/<org>/<plugin>/<upstream>' (matches buildPluginToolRef). (#8)
- tests: shared tsc 0; ref 5/5; provider-layer 7/7.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…e contracts

Round-2 review: the shared schemas existed but weren't the runtime contract,
and the devices write-path/cloud + service-shaping were still in the old world.

zai-org#1/zai-org#2 devices WRITE-path → shared camelCase (master plan §5.1.1/§8.3):
- shared/schemas/devices.ts gains app-facing input contracts (camelCase):
  CreateCloudDeviceInput, StartPairingInput, ClaimDaemonServiceInput,
  SetActiveDeviceCapabilitiesInput + CreateCloudDeviceResultView /
  ActiveDeviceCapabilitiesView.
- controller parses these; device-sdk + web send camelCase bodies
  (device_type→deviceType, service_kind→serviceKind,
  remote_agent_machine_id→remoteAgentMachineId,
  device_capability_ids→deviceCapabilityIds, workspace_id→workspaceId,
  host_provider→hostProvider).
- access-bindings POST now on the shared SetActiveDeviceCapabilitiesInput
  (the wire-shape test reframed to the app-facing contract); GET list key →
  deviceCapabilityIds.
- cloud.ts result → camelCase; sandbox docker-backend + its test updated.
- device-runtime startPairingSession consumes the shared camelCase
  StartPairingInput + DevicePairingTicketView.
- removed the now-orphaned device-protocol input/result schemas
  (CreateCloudDeviceInput/Result, StartPairingInput, PairingTicket,
  ClaimDaemonInput, SetActiveDeviceCapabilitiesInput); kept the genuine wire
  ConsumePairing*/ScopedSubjectTargetWire.

zai-org#3 model-groups + mcp-plugins controllers → sendData(ViewSchema, value):
- 30 model-groups + 8 mcp-plugins sites now parse the shared View schemas at
  runtime (the schemas were "dead definitions"). Envelope → { data }.
- updateModelGroup always returns ModelGroupDetailView (single schema for PUT).
- web lib/api.ts unwraps { data } in-method, keeping each method's public
  shape stable so callers are unchanged. Endpoints without a View schema
  (auth-session/audit/install-plan) deliberately left as-is.

zai-org#5 devices service no longer shapes DTOs: listDevices/getDevice/startPairing/
claimRemoteAgentDaemon return domain records (DeviceSummaryRecord/
DeviceDetailRecord/DevicePairingTicketRecord/DeviceServiceRecord); the
controller calls the presenter. Added presentDevicePairingTicket.

zai-org#4 mcp-plugins schemas: z.custom already gone (r1); documented the genuinely
plugin-author-owned opaque JSONB (configSchema/configData/toolsManifest/
metadata) — the structured fields are already nested-schema'd.

zai-org#7 device-sdk tests: added src/index.test.ts (0 → 8 tests) locking the
camelCase send/parse boundary + the snake wire consume shape; build moved to
tsconfig.build.json so tests stay out of dist.

Verification: full monorepo build green; api 1640 pass/0 fail/2 skip; guards
(layering/db/fk/soft-delete/datetime) clean; device-sdk 8/8, device-runtime
314/315(+1 skip), remote-agent-daemon 24/24; web + mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ervice→presenter retreat

Round-3 review: app-facing endpoints still on bare reply.send, /auth/me unmigrated,
mcp-plugins auxiliary flows uncollected, and service still produced outward Views.

zai-org#1 devices controller → sendData { data } (§8.3): list/detail/pairing/claim/cloud
now sendData(ViewSchema, ...) → { data: ... }. device-sdk read methods unwrap
res.data + parse the shared view; device-sdk tests updated to { data } fixtures.
204 endpoints (delete/detach) unchanged; consume/bootstrap stay bare wire.

zai-org#2 access-bindings GET → sendData(ActiveDeviceCapabilitiesViewSchema, ...) →
{ data: { deviceCapabilityIds } }. POST stays 204 on the shared input contract.

zai-org#3 mcp-plugins auxiliary endpoints collected to sendData with new shared schemas
(PluginAuthSessionView/Envelope, PluginInstallPlanView/Envelope, PluginAuditLogList):
install-plan, auth-session start/get/inspect, audit tool-calls/events. Only the
OAuth HTML callback + { error } envelopes remain bare. install-dialog unwraps
res.data for the auth-session methods.

zai-org#4 /auth/me GET+PUT → sendData(AuthMeViewSchema, { user, session }) → { data }.
New shared schemas/auth.ts (UserProfileView/AuthSessionSummaryView/AuthMeView).
web lib/api.ts getMe/updateMe + lib/server-auth.ts read res.data.{user,session}.

zai-org#5 service→presenter retreat (controller owns presentation) across 11 app-facing
modules (organization/files/relationship/memory/remote-agents/workspace/
workspace-apps/automation/chat/model-groups/mcp-plugins): service.ts now returns
DOMAIN RECORDS; the controller calls present*() before each send (envelopes
unchanged). present*() in service.ts: 106 → 65 (remainder are nested-record
assembly + cross-module-consumed records + Tier-D timestamp wrappers, permitted
by the layer-matrix JUDGMENT clause). New record types in repo.types/presenter
per module. Fixed 2 new r7 (return { ...row }) violations the retreat introduced
(remote-agents/workspace-apps) via const-assignment.

Tier-D / non-HTTP modules (session/context/tasks/tool-call-tasks) excluded per
plan ("不强制 presenter").

Verification: full monorepo build green; api 1640 pass/0 fail/2 skip; guards
(layering/db/fk/soft-delete/datetime) clean; device-sdk 8/8, device-runtime
314/0fail, remote-agent-daemon 24/24; web + mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
… guard close

Round-4 review: cloud-input schema stricter than server; relationship/remote-agents
controllers still bare; access-bindings half-migrated; TableRow leaked outside repo
files (guard couldn't prove repo-only DB access); §5.3 split-controller/appRoute
mechanism + guard rule 5 entirely unadopted across mixed modules.

zai-org#1 CreateCloudDeviceInputSchema: title/hostProvider → optional (the server
applies defaults; the strict schema would reject requests the server accepts).

§5.3 (subsumes zai-org#2 + zai-org#3) — adopt appRoute/wireRoute across ALL mixed modules:
implemented guard rule r5 (mixed modules forbid bare app.<verb>; must use
appRoute/wireRoute/split-controller). Migrated every bare route in devices,
mcp-plugins, runtime-authorizations, files, relationship, remote-agents,
automation, im — APP routes → appRoute (handler returns value; helper wraps via
sendData → { data }), WIRE (handshake/webhook/binary/HTML/WSS/reverse-MCP) →
wireRoute (bare). devices/mcp-plugins were pure registration swaps (already
sendData, no envelope change); the bare modules went bare→{data} with new shared
View schemas (automation/files/im/relationship/remote-agents/runtime-
authorizations.ts) + web/mobile consumer unwraps in lockstep. Guard r5 baseline
now 0. Fixed appRoute to PRESERVE a handler-set status (reply.status(201)) — the
prior code let sendData's default 200 clobber 201s; added route.test.ts locking
the { data }/201/204/wire contract.
zai-org#3 access-bindings GET query → camelCase (subjectKind/subjectActorId/…); route
already on appRoute + { data } response.

zai-org#5 repo-boundary: extended guard r2 from service/controller-only to ALL non-repo
files (only repo*.ts/repo.types.ts may use TableRow). Moved the TableRow/
TableInsert/TableUpdate aliases out of 12 helper files (access/binding-storage +
subject-registry, im/service/*, mcp-plugins/{plugin-auth-connections,mijia},
memory/indexing, session/runtime, tasks/action-tokens) into per-module
repo.types.ts. guard r2 baseline now 0. Also: guard now strips comments before
matching (kills doc-comment false positives).

GUARD-LAYERING BASELINE IS NOW FULLY EMPTY (0 grandfathered entries across all 6
rules: r1/r2/r3/r4/r5/r7) — the layering is genuinely enforced, not just passing.

zai-org#4 (service→presenter) was completed per-plan in round-3; the 64 remaining
present* are the JUDGMENT-permitted nested-record/cross-module/Tier-D cases.
zai-org#6 (web runtime schema.parse) not required by plan (device-sdk is the parsing
consumer); web stays a typed thin client.

Verification: full monorepo build green; api 1646 pass/0 fail/2 skip (+route.test);
guards (layering[empty]/db/fk/soft-delete/datetime) clean; device-sdk 8/8,
remote-agent-daemon 24/24; web + mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…in AGENTS.md

Round-5 review re-flagged 4 items; an exhaustive re-audit shows 3 are already
resolved and 1 is at its intended end-state:

zai-org#1 CreateCloudDeviceInputSchema strictness — STALE: fixed in round-4 (fddca43);
   title/hostProvider are .optional() and device-sdk parses the same schema it
   sends, so SDK and server accept identical bodies. Verified, no change needed.
zai-org#3 TableRow outside repo — RESOLVED in round-4: the guard r2 baseline is empty
   ([]); all DB-row types live in repo*.ts/repo.types.ts; the guard now strips
   comments so doc-comment mentions aren't flagged. The reviewer's "few left,
   mostly comments" predates that. No change needed.
zai-org#4 web runtime schema.parse — OUT OF PLAN SCOPE: the master plan classifies
   web-next as an app client and device-sdk + the API's sendData/appRoute as the
   runtime-parse boundary. web is a typed compile-time-contract client by design;
   the 6 remaining snake reads are RFC8628 OAuth device-flow wire fields, and the
   3 any are memory request-body accumulators (no shared input schema). No change.
zai-org#2 service→present* (64 sites) — ALL PERMITTED: a fresh per-site classification
   of every present*() in service.ts found exactly 0 clean violations. Each is
   one of: (a) a row→record / record-assembly mapper producing an in-api
   XxxRecord (e.g. presentFileAsset→StoredFileRecord, presentActorRow→Actor,
   presentUser→User), (b) a thin presentInstant/presentOptionalInstant timestamp
   wrapper, or (c) a record consumed cross-module (automation rules read by
   ai/session-tools; remote-agents snapshot read by chat; etc.). The lone
   HTTP-body case (model-groups getModelGroup→ModelGroupDetailView) is also
   consumed in-api as a record by the seeder + ownership pre-handlers +
   updateModelGroup, so it's the same cross-consumed-record exception. The
   master-plan JUDGMENT clause + Tier-D exemption permit all 64.

To stop future reviews re-flagging this as "unfinished", codified the rule in
AGENTS.md (七层职责): service may call present* for row→record assembly,
timestamp wrapping, and cross-module records; what's forbidden is service
producing+bare-returning the OUTWARD app DTO as the sole HTTP-body source — that
stays in the controller via appRoute/sendData.

Verification: full build green; api 1646 pass/0 fail/2 skip; guards
(layering[empty]/datetime) clean; device-sdk 8/8; web + mobile typecheck clean.
No functional code change → running containers already on the verified r4 state.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…, live-verified

External review R4 zai-org#1 (P0) + design §1.4/6.3/6.4/6.5/6.8. A cubesandbox turn silently lost ALL work (base never pushed to the VM; commit scanned the unchanged host scratch; teardown DELETEd the VM before commit). Fixed + proven live 6/6 against real envd.

- WorkingSetTransport abstraction: createDetachedWorkingSetBridge (the CI-proven delete-aware mirror + stat-cache) now takes a transport, not docker-exec+containerId. docker-exec transport keeps the S12 path byte-identical; new envd transport (cubesandbox/working-set.ts) over the plane's RemoteEnvdTransport.
- F1 delete-propagation on PULL: fetchChangedIntoMirror was additive-only (a VM-deleted file resurrected from the base). Now prunes the mirror to exactly the VM listing before scanCommitDir.
- F6 full-ms mtime for envd (was second-truncated -> a same-size same-second edit was a false cache hit -> lost update). 2b/2d: envd list == find -type f (regular-only, dotdirs/dotfiles), all keys VFS-rooted.
- Provision PUSH: off-box materialize -> applyManifest replicates the base INTO the VM.
- Teardown reorder (teardownOffBoxSandbox): durable CAS close-gate (abort-if-lost, zai-org#5-A) -> drain the original plane (no kill) -> reconnect token-bearing -> PULL (delete-pruned) -> commit -> GATE the VM DELETE on pull+commit success. On failure: KEEP the VM, mark mounts failed, leave state='closing' for recovery re-pull -- never kill unpulled work. (state='closing' before terminal writes satisfies the R3.8 trigger.)
- F2 recovery: recoverFailedSandboxMounts off-box branch reconnect->pull->commit->THEN kill; ensureRuntimeStoppedForRecovery never kills an unpulled off-box runtime; plus a race-guard so a mount-less closing-reaper re-drive with pending failed mounts defers instead of rm-ing preserved work.
- EMPIRICAL: a PAUSED cube VM's envd is unreachable (504), so the design's pause-then-pull would 504 the pull; pull runs on the drained-but-live VM, pause skipped (residual bg-writer non-freeze accepted).

Gates: typecheck 0, sandbox 248/0 (full-turn round-trip + delete-propagation + F6 + docker-vs-envd list-parity), guards clean, verify:boundary green (api 2864/0). LIVE cube full-turn 6/6: base pushed + edit + create round-tripped + VM-DELETE PROPAGATED.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
… P0 security)

External review R5 zai-org#1 (reproduced: a cross-session HOST write). The off-box working-set
transport mapped envd-returned entry paths onto host (mirror) and VM filesystem sinks
with only a vmToVfs prefix-strip + startsWith('/') — never the API-side canonicalization
the tool plane applies to the SAME untrusted-VM surface (the agent runs code in the VM;
envd is untrusted). A '..'-bearing envd path normalized through join(sandboxRoot, rel)
into a sibling session's dir.

Design-review correction folded in: do NOT reuse canonicalVfsPath — it is a DOS/Windows
validator that THROWS on legal Linux names (aux/con/com1/trailing-dot/':'/'\'), which
would DROP legitimate base files -> prune them from the mirror -> commit spurious deletes.

- scopeWorkingSetKey: POSIX-only normalize (collapses ./.. , drops '..' past root) +
  mount-VFS-root scope check. Returns the canonical key or null (escape/non-absolute).
- listOne validates the UNTRUSTED envd key BEFORE it is used as a recursion root OR a
  mirror key; an escaping entry is DROPPED (logged), never dialed. A malicious DIRECTORY
  entry is likewise not recursed into.
- Depth: the old silent return at depth>64 (which dropped every deeper file -> pruned
  the mirror -> committed spurious deletes) is now a fail-loud WorkingSetTransportError,
  and the bound is raised to 4096 (real dir-cycle protection, not a working-set cap).
- Belt-and-suspenders: assertUnderMirror (resolve) on every mirror write/read/remove, and
  assertUnderVmRoot (posix.normalize FIRST — a string-prefix check passes '/workspace/..')
  on every raw envd read/write/remove leg (closes the in-VM boundary erosion too).

+4 unit tests: the reported attack is now DROPPED; an escaping dir is not recursed; the
belt rejects a lowered VM path escaping the root. A real (honest) envd cannot inject the
'..' primitive, so the stub test is the faithful reproduction. typecheck 0, verify:boundary
green (2904/0).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ff-box

The off-box working-set transport listed only regular files: the envd walk
(listOne) recursed into directories but emitted no entry for them, and the
mirror walk (walkMirror) emitted only files. An EMPTY directory an agent
created in the VM was therefore lost on PULL (never reached the mirror →
never committed) and an empty base directory was never PUSHed to the VM. The
CAS manifest DOES model empty dirs (fs-helper ManifestEntryKind file|dir),
so this was a transport-layer gap, not a manifest limit.

Scope (descoped per the R5 plan): empty-dir round-trip only. symlink + mode
fidelity remain a documented off-box non-goal (envd lacks the primitives);
the depth cap was already made fail-loud in Phase B1.

- ContainerFileStat gains kind:'file'|'dir'. A 'dir' entry is emitted ONLY
  for an EMPTY directory (a non-empty dir is implied by its files' parents);
  the mount root is never emitted (an empty mount = an empty manifest).
- WorkingSetTransport gains makeDir (docker `mkdir -p`; envd makeDir with a
  409 already-exists swallowed — idempotent).
- fetchChangedIntoMirror splits the listing into files (stat-cache reconcile
  + fetch — a dir has no bytes to read/hash) and dirs (mkdir'd into the
  mirror so scanCommitDir commits them). The prune runs files-first then
  empty-dirs: a mirror empty dir absent from the VM's dir set is rmdir'd
  (delete-propagation). planFor diffs FILES only so a container dir entry can
  never land in the PUSH `removes`.
- pushMirrorToContainer replicates empty mirror dirs into the VM via makeDir.
- The cube bridge wires makeMirrorDir / listMirrorEmptyDirs / removeMirrorDir,
  each belt-asserted under the mount (the same zai-org#1 traversal guard as files).

Parity fixture has no empty dirs, so the docker-vs-envd list-parity test is
unaffected. Tests: envd empty-dir emission (+ mount-root exclusion) and a
bridge round-trip (PULL mkdir, PUSH mkdir, prune rmdir). verify:boundary
green (2912/0).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…rbitrary host write)

R6 review P0: CAS materialize() recreates a base-snapshot symlink as a REAL host
symlink inside the off-box mirror; the R5 assertUnderMirror belt is a purely
LEXICAL resolve()+prefix check (zero fs access), so the teardown PULL's mkdir /
writeFile FOLLOW the symlink and write OUTSIDE the mirror as the shared API
process (reviewer reproduced {"escaped":"PWN"}). Reachable cross-backend: a
docker/device turn can commit `ln -s /etc evil` into the shared base snapshot.

Fix — no-follow the mirror write legs (design-reviewed: neutralize, don't
preserve; keep walkMirror symlink-free):
- firstSymlinkComponentUnderMirror: walk the target's components UNDER the mount
  top-down with lstat, stopping at (returning) the first symlink — so a symlink
  at ANY depth is detected without ever following one (lstat only no-follows its
  last component; returning at the first symlink guarantees every lstat'd path
  has only real-dir ancestors).
- writeMirrorFile / makeMirrorDir: neutralizeMirrorSymlink UNLINKs the first
  symlink component before mkdir/writeFile, so the write lands on a REAL node
  inside the mount — the actor's bytes stay inside the mount (no legit data loss)
  and the weaponized symlink is dropped (off-box can't round-trip symlinks anyway,
  H-7); unlinking it before scanCommitDir also purges it from the committed tree
  rather than re-recording the escape primitive.
- readMirrorFile: assertNoSymlinkOnReadPath refuses to read THROUGH a symlink (a
  host-file exfil into the VM/CAS).
- walkMirror is UNCHANGED (it is shared by PUSH's listMirrorFiles — emitting
  symlinks would either abort provision or stream a host file into the VM); it
  already skips symlink dirents, so PUSH never reads through one.

The write-time unlink + the zai-org#2 refreshSpaces gate (Phase C, removing the only
mid-turn mirror materializer) mean the sole symlink source is the pre-PULL base
materialize, so the sequential single-threaded writes have no concurrent
symlink-inserter to race (openat2 via fs-helper is the deferred gold standard).

Tests: reproduce the escape → the VM file lands INSIDE the mirror (symlink
replaced by a real dir), nothing written to the symlink target; PUSH never
exfiltrates the host secret behind a mirror symlink. verify:boundary green.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
… 502-confirm + H-6/H-3 guards + H-1/H-7 honest docs

R6 review P2 hardening — six findings on the off-box path.

zai-org#6 — pull was optional (Promise<void>) on WorkingSetBridge; OffBoxSandboxAdapter did
not narrow workingSet(). New OffBoxWorkingSetBridge extends WorkingSetBridge with a
REQUIRED pull(): PullOutcome; OffBoxSandboxAdapter.workingSet() returns it; the cube
factory + the teardown/recovery call sites drop their `if (bridge.pull)` guards —
"the adapter forgot pull" is now a type error, not a silent no-op.

H-8 — a CubeProxy gateway 502/503/504 mapped DIRECTLY to SandboxResourceGoneError, so
a transient proxy blip forced an irreversible teardown of a live VM. The gone-mapper
now takes a control-plane confirmGone() (getInfo tristate) and declares gone ONLY on a
CONFIRMED-dead VM; a live/unknown/unconfirmable probe re-throws the ORIGINAL retryable
error. An envd-level 4xx (VM up, op rejected — e.g. a 404) never consults it.

H-6 — a default-empty SANDBOX_DEPLOYMENT_ID on a shared/authenticated Cube account made
the orphan sweep the "empty-id owner", cross-reaping sibling deployments' live VMs. The
cube config validate now REJECTS an empty deployment id whenever an api key is set (an
unauthenticated self-hosted single-tenant deploy still needs none).

H-3 — provisionSandbox's double-create back-off resolved the in-flight runtime via
runtimeIdFromMounts (mount.sandbox_id), which is NULL during the back-fill window; the
guard then missed the in-flight provision and tore down the first worker's fresh VM
(the double-create → unsandboxed turn). It now resolves the 'provisioning' row by
session_id (getSandboxBySessionForControl), seeing it the instant it exists — the 409
stays retryable.

H-1 — the off-box teardown doc claimed the 5s drain was "the writer-freeze we
guarantee". Corrected: the drain bounds only the remote calls WE issued, NOT VM-side
execution; with no pause/suspend API, a VM process outliving its exec call may write
during the PULL, which is therefore a point-in-time snapshot (per-file atomic, merged
by commit's 3-way), not a frozen one.

H-7 — documented the off-box fidelity limitation: symlinks do NOT round-trip (neutralized
+ excluded — SECURITY-load-bearing per zai-org#1; preserving them reopens the mirror-escape
hole), and the exec bit is not preserved (the pull writes host-default mode; the CAS
manifest carries mode, so it is fixable by threading envd FileEntry.mode through the
transport — deferred as a P2 fidelity gap, not plumbed into the security-sensitive
write path here). Host planes are unaffected (their mirror is the live dir).

Tests: +7 H-8 (502/503/504 × confirm true/false + a 404 passthrough) + 3 H-6
(api-key⇒id-required / id-present-ok / unauth-ok). zai-org#6 is enforced at the type layer;
H-3 is a like-for-like resolver swap covered by the existing provision path; H-1/H-7
are doc-only. verify:boundary green (sandbox+cube+config 336/336).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…PULL (live-verify finding)

The R6 live cube verify (scripts/r6-cube-verify.ts) caught a confinement leak the zai-org#1
WRITE-side fix did not cover. The off-box working-set walk enumerates the VM via envd
listDir and excludes non-regular files with `if (e.type !== "file") continue`. But the
REAL CubeSandbox envd STAT-FOLLOWS symlinks: a VM symlink is reported with the TARGET's
`type` — a symlink→/etc/passwd comes back as type:'file' (+ the target's size), and a
symlink→dir as type:'directory'. So the type filter did NOT exclude it:

  • a file-symlink was READ THROUGH → /etc/passwd content pulled into the mount's mirror
    and committed to that file_space's CAS (out-of-mount content exfiltrated in);
  • a dir-symlink would have been RECURSED INTO → an entire external tree (e.g. /etc)
    pulled into the mount.

The live run confirmed it: pulled=[…/big.bin, …/evil-link, …/normal.txt] and /etc/passwd
content present in the mirror.

Root cause + fix: envd's `type`/`mode` are stat-followed (unreliable for symlink
detection), but the `permissions` field is LSTAT-based — a symlink's symbolic mode leads
with 'l'/'L' ("Lrwxrwxrwx", confirmed by probing the live envd). The walk now EXCLUDES
any entry whose permissions reveal a symlink, BEFORE the dir-recursion, so both a
file-symlink (no read-through) and a dir-symlink (no recurse) are dropped — the read-side
twin of neutralizeMirrorSymlink. The pre-existing `type !== 'file'` check is kept
(catches an envd that correctly reports type:'symlink'); this is defense-in-depth over it.
Docker's transport uses `find -type f`, which never follows symlinks, so only the envd
path was affected. Symlinks do not round-trip off-box anyway (H-7).

Test: a stat-following symlink (type:'file'/'directory' + permissions:'L…') is excluded
and its target never read/recursed. Re-verified live: 7/7 (oversize streams whole; the
symlink is excluded; no /etc/passwd leak). verify:boundary green (cube 77/77).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ter is the sole lifecycle layer

R7 clean-slate (finding zai-org#1, the crux). SandboxBackend was a SECOND lifecycle interface
(create(spec) + connect(ref)) that SandboxAdapter already declared identically; the
resident adapters merely PASS THROUGH to create*Backend factories (`const backend =
createLocalSandboxBackend(...); create: (s) => backend.create(s)`). Two abstractions for
one job — pure maintenance burden. (R5/R6 wrongly called this "load-bearing"; only the
shared TYPES are — the wrapper interface + factories are not.)

Collapse (no old code / no old names / no compat layer):
- Delete the SandboxBackend interface. SandboxAdapter is now the SOLE lifecycle
  abstraction (it always carried create/connect directly).
- createLocalSandboxBackend → free functions provisionLocalSandbox(spec, deps) +
  connectLocalSandbox(ref). createDockerSandboxBackend → provisionDockerSandbox(spec,
  opts). createDockerReconnectBackend is DELETED — its connect() was byte-identical to
  the provision factory's connect(), so both collapse into ONE connectDockerSandbox(ref,
  {spawnImpl}); its throwing create() is gone (a connect-only object no longer exists).
  The adapters call these functions directly.
- F-A env-free docker reconnect is PRESERVED — now STRUCTURAL: connectDockerSandbox
  takes no provision options, and provisionDockerSandbox reads
  dockerBackendOptionsFromEnv() LAZILY inside create(). Teardown never touches the
  provision env.
- Renames (no old names): SandboxBackendError → SandboxAdapterError;
  DockerSandboxBackendOptions → DockerSandboxOptions; and the FILES
  sandbox-backend.ts → sandbox-lifecycle.ts, docker-sandbox-backend.ts → docker-sandbox.ts
  (git mv; ~24 importers repointed). Reworded every "backend" comment that named the
  deleted abstraction to "adapter" (the content-storage `backend`/`local_cas` is a
  different concept and stays).
- Tests: sandbox-lifecycle.test.ts / docker-sandbox.test.ts drive the free functions via
  a thin {create, connect} test helper (the adapter's real shape); the obsolete
  "reconnect backend create() is unsupported" test is deleted.

The shared contract TYPES (SandboxHandle/Spec/Ref/Info/Liveness/DataPlaneCredentials +
requireHostSpec + pid-identity helpers) stay in sandbox-lifecycle.ts — used by every
adapter. verify:boundary green (sandbox+cube 315/315).
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.

2 participants