fix(sdk): harden hosted loader, lock names, and base matching - #583
Conversation
Three findings left unresolved when #552 merged. Resolve the host's @relayflows/surface entry once while hosted-extension-sandbox.ts initializes. require.resolve consults the writable Module._resolveFilename, so resolving lazily ran after authored flow code could replace it and hand the sandbox a different package. Require a hosted lock's plugin name to be one lowercase kebab-case path component. It reaches pluginStoreDirectory as a path segment, and only source.owner/repo/sha/path were pattern-checked, so a name containing ../ resolved outside the plugin store. Restore first-match selection in assertBaseCompatible. The intrinsic-capture rewrite dropped the loop's exit, so a later duplicate compat.base name decided the version range instead of the first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Session-Id: 7d116c0d-6756-4905-ace7-1235559066ed
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review Independent exact-head security review requested for |
|
To use Codex here, create a Codex account and connect to github. |
Summary
Three findings were still unresolved when #552 merged (
d401cb8c, merge78cc5556). This is the minimal fix for all three, in shipped code paths the native Babysitter artifact depends on.hosted-extension-sandbox.ts:254).resolveSurfaceRootcalledcreateRequire(import.meta.url).resolve('@relayflows/surface')at dispatch time.require.resolvegoes throughModule._resolveFilename, which is writable, so authored flow code loaded earlier in the host process could point the sandbox at a different package. The entry is now resolved once while the module initializes, before any authored module can load. A failure is still reported when a caller needs the root.hosted-extension-declarations.ts). The hosted lock parser pattern-checkedsource.owner,source.repo,source.shaandsource.path, but accepted any string forname— andpluginStoreDirectoryjoins that name into the store path as${name}@sha256:${digest}. A lock naming../escaperesolved outside the plugin store.namemust now be one lowercase kebab-case path component.compat.baselast-vs-first match (Cursor,flow-extension-compat.ts). The intrinsic-capture rewrite in feat(sdk): isolate hosted Babysitter capability #552 replaced.find()with a loop that has no exit, so the last duplicate name decided the version range. First-match semantics are restored, matching the documented behaviour.validateFlowExtensionManifestrejects duplicate base names, so this is defence in depth for callers that build a manifest another way.Evidence (
evidence/hosted-hardening/)tests.txt:npm run typecheck,npm run build,npm run typecheck:testsallExit: 0. New suite 11/11.mutation.txt: each fix reverted, the specific test captured failing, the file restored byte-for-byte (sha256 printed before and after), and the test captured passing again.../escape,a/b,.and...hosted extension isolation requires Linux(they need Linux + bubblewrap), and the one deep-equal failure reproduces with every change stashed, captured intests.txt. Linux CI is the authority.Scope
No behaviour change for the merged Babysitter artifact:
extensions/babysitteris untouched and stays byte-identical tod3ee3b55(digestbdf2187b…, manifest5631a06b…). #549's in-process refusal is untouched.BABYSITTER_REFand the pinned base-source hash are unchanged.🤖 Generated with Claude Code
Note
High Risk
Changes affect hosted-extension sandbox trust boundaries (module resolution), path validation for lock-driven store layout, and extension base compatibility semantics—security-sensitive loader and path handling.
Overview
This PR hardens three hosted-extension / compat paths left open after #552: sandbox Surface resolution, lockfile plugin names, and
compat.basematching.Surface loader:
@relayflows/surfaceis resolved once athosted-extension-sandboxmodule load (viaSURFACE_ENTRYandcapturedSurfaceEntry()), so later tampering withModule._resolveFilenamecannot redirect the sandbox;resolveSurfaceRootuses that cached path instead of resolving at dispatch time.Lock names: Hosted
flows.lock.jsonpluginnamemust match lowercase kebab-case (LOCK_NAME); traversal, slashes,./.., and invalid casing are rejected withplugin_lock_invalidbeforenameis used as a plugin-store path segment.Base compat:
assertBaseCompatiblestops at the firstcompat.baseentry with a matching name (restoring pre-rewrite.find()behavior), so duplicate names cannot widen or narrow the version range.A new
hosted-hardening.test.tssuite (11 tests) covers these behaviors;evidence/hosted-hardening/adds mutation logs (revert each fix → targeted failure → restore) and broader test run notes (macOS Linux-only suite failures called out as pre-existing).Reviewed by Cursor Bugbot for commit b37e4c7. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes three leftover findings from #552 in
packages/sdk: the hosted surface entry resolves before authored code can steer the loader, the hosted lock name is validated as a single path component, andcompat.basematching follows documented first-match semantics.extensions/babysitteris untouched and stays byte-identical to the previous release.Bug Fixes
@relayflows/surfaceonce at module init;require.resolveconsults the writableModule._resolveFilename, and resolving lazily let authored flow code swap the package the sandbox loads.nameto be one lowercase kebab-case path component; it previously accepted any string and joined it into the plugin store path as${name}@sha256:${digest}, so a name like../escaperesolved outside the store.compat.baseselection; the intrinsic-capture rewrite dropped the loop's exit, so the last duplicate name decided the version range.Written for commit b37e4c7. Summary will update on new commits.