chore: remove unused legacy assets and add open-source housekeeping - #1
Merged
Merged
Conversation
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).
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.
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 byrelay/managed-command-providers.json. CI cache-key references inrelay-ci.ymlandrelay-gui-build.ymlare updated to the new manifestin 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 seedanceskill at
.agents/skills/seedance2-skill/SKILL.mdis methodology, not aconsumer of these prompt files.
docs/relay-auto-skills.md— internal note. Code mentions ofrelay-auto-skillsresolve to./relay-auto-skills.jsor 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:.ideafor JetBrains users.Test plan
relay/managed-command-providers.json).