Repository navigation
fix: close the plugin findings of the S2 security review - #96
Merged
lr00rl merged 5 commits intoOct 17, 2026
Merged
Conversation
…hers The staging decision asked only whether the written record was itself fleet-bound before or after the write. A provider record that a fleet-bound collection gathers, as an explicit member or by a member tag, is not fleet-bound, so an edit of it, a new record carrying a member tag, or a tag added to or dropped from a live record took the live branch. Core serves a fleet-bound collection's provider nodes to every identity holder without validating them, so the next render handed the new nodes out with no plan (S2-SR-1). writeContext.stage now also stages when a collection that is fleet-bound over the union of live and staged state gathers the record at its live facts or at its new facts. That one rule covers a member edit, a tag join, a tag added or removed, a new or restored record under an id a collection already lists (a rename lands that way, since membership follows ids), a member that stops being a sub, and batch writes from import and migrate. deleteSubscription refuses such a record with fleet_delete_requires_plan and names the collections, as it does for a fleet-bound record. Core's closure and plan need no change: depends_on already reports the dependents and the plan already folds provider content. Rejected: staging whenever the record has a pending staged revision. It would stage live edits of a legacy record whose migration is staged, which the editor depends on. Tested: the nine new TestFleetGathered* write cases fail with the fix reverted (saved=true staged=false, delete not refused) and pass with it; go test ./... in system-go passes.
…dentity The script network is granted per method, and render carries it, so a Script Operator or Resolve Domain step on a fleet record or a fleet-bound collection ran with the network while processNodes handed it every node with its Lattice block (node id, line hash id, groups, chain, addresses) and a credential placeholder that names the line. The author needs only substore:admin, which does not carry vpncore:read, and the step also runs when core previews a staged revision for a plan, before approval (S2-SR-2). A render that reaches fleet nodes in any state (the record fleet-bound live, or over the union of live and staged facts with the rendered revision added) and the editor's fleet previews now run with the network withheld: withholdScriptHTTP installs a gateway that sends nothing and counts every request, and the call fails with fleet_script_network_unavailable, leading its message, whenever a script asked, even if the script caught the refusal. processNodes withholds it on its own as well, so no path that runs a script over fleet nodes can reach the network. The whole render is withheld, including a provider member's own chain inside a fleet-bound collection, which matches fetch, where no chain has a network. processNodes no longer sends a fleet node's Lattice block, and it swaps each credential placeholder for an opaque per-call token, the same token for every node of a line, and swaps it back on return. Line recovery and the row's block are re-attached outside the script exactly as before, so binding is unchanged. A provider node keeps only its per-computation provenance mark, which names no line. The UI learns the new code so errorCodeOf recognises it. Tested: the seven new fleet script network tests fail with the fix's two mechanisms reverted (one request sent through the host per render, the script reads the Lattice block) and pass with it; the provider render control keeps its request. go test ./... in system-go, -race on the touched tests, and the UI unit tests and typecheck pass.
migrate_record reads the catalogue and its reply warns whether a pinned root is in the catalogue and whether a name-based step matches any line label. A caller with substore:admin and no vpncore:read could ask one question per call about line labels and line identifiers (S2-SR-4). The migration flow relies on those warnings, so the method keeps them and declares the scope instead, and the exemption in TestProxyMethodsDeclareVpncoreRead goes. The test now also names migrate_record among the methods that must declare the scope. A token without vpncore:read no longer sees migrate_record in the handshake. Tested: TestProxyMethodsDeclareVpncoreRead fails with the manifest change reverted and passes with it; the manifest tests pass and lattice-plugin-manifest-check at the CI pin accepts the manifest.
collectionParts reused a provider member's snapshot block whenever the block's source matched, whatever revision the member rendered at. Before S2-SR-1 a provider member never had a staged revision of the same source, so that was safe. Now every edit of a member a fleet-bound collection gathers stages, and core previews the collection with that member named at its staged revision. The preview took the live block, the plan's dependent section answered added 0, changed 0, and the approver approved a change the plan did not show; the next fetch then served it. A provider block is reused only when it was built at the revision the member renders at. A member at a staged revision resolves its staged content live, as a member switched from fleet to provider already did. A member at its live revision still reuses its block, so core's byte-for-byte comparison at the decision is unchanged for it. Tested: TestFleetGatheredStagedMemberPreviewsItsStagedContent fails with the revision check removed (the preview shows the live provider node only) and passes with it; the staged collection render tests pass.
TestFleetScriptSeesNoLineIdentity covered vless only, whose placeholder sits in uuid. It now renders one line per bind-shape protocol (vless, trojan, anytls, hysteria2, tuic, vmess, socks), so a placeholder in password or username, or one kept in a socks userinfo, would reach the script and fail the test. With the masking alone removed the test fails.
lr00rl
merged commit Oct 17, 2026
82d7a34
into
feature/20261010_s2-fleet-sources
5 of 6 checks passed
This was referenced Oct 17, 2026
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.
Fixes the plugin findings of the S2 security review (design 28, review recorded in LatticeNet/lattice#23): S2-SR-1, the plugin part of S2-SR-2, and S2-SR-4, plus one sibling of S2-SR-1 the fix exposed. The server part of S2-SR-2 (a hash of the process chain in the plan section) and S2-SR-3 belong to the server lane; the contract this PR needs from core is at the end.
S2-SR-1 (High): writes that change what a fleet-bound collection gathers
writeContext.stage(system-go/store_guards.go) asked only whether the written record was itself fleet-bound, so a provider record that a fleet-bound collection gathers went live, and core serves such a collection's provider nodes to identity holders unvalidated. The staging decision now also stages when a collection that is fleet-bound over the union of live and staged state gathers the record at its live facts or at its new facts (gatheredByFleet,fleetGatherers). It covers save, import, migrate and restore, because they all go throughdecideWriteorwriteContext.stage.deleteSubscription(system-go/store_archive.go) refuses a gathered record withfleet_delete_requires_planand names the collections.Sibling found while checking the fix against core:
collectionParts(system-go/subscription_fleet_render.go) reused a provider member's snapshot block whenever the source matched, so core's preview of a collection with that member at its new staged revision still showed the live block. The plan's dependent section answeredchanged: 0and the approver would approve a change the plan did not show. A provider block is now reused only when it was built at the revision the member renders at.S2-SR-2 (Medium), plugin part: scripts on fleet-bound records
system-go/fleet_script_network.go).withholdScriptHTTP(system-go/substore_engine.go) installs a gateway that sends nothing and counts every request (system-go/substore_script_http.go). The call then fails withfleet_script_network_unavailable, leading its message, even when the script caught the refusal, so a step that needed the network is never skipped silently.processNodesalso withholds the network on its own, so no path that runs a script over fleet nodes can reach it. The whole render is withheld, a provider member's own chain inside a fleet-bound collection included, which matches fetch, where no chain has a network.processNodes(system-go/substore_engine_process.go) no longer sends a fleet node's_latticeblock. It swaps every credential placeholder, which carries the line uuid, for an opaque per-call token (the same token for every node of a line), and swaps it back on return. Line recovery and the row's block are re-attached outside the script as before, so binding is unchanged. A provider node keeps only its random per-computation provenance mark, which names no line.ui/src/client.ts), soerrorCodeOfrecognises it.S2-SR-4 (Low):
migrate_recordmigrate_recordnow declaresvpncore:readinmanifest.json, and the exemption inTestProxyMethodsDeclareVpncoreReadis gone. The test also listsmigrate_recordamong the methods that must declare the scope.Done
system-go/store_fleet_gathered_test.go. These cover the reviewer's two probes (member edit, tag join) and one test per sibling write kind: tag added, tag removed, a record saved under an id a collection lists (how a rename lands, since membership follows ids), a member that stops being a sub, delete (explicit member and by tag), restore, and batch import. A negative case covers provider-only collections. With the fix reverted, the nine write cases fail (saved: true, staged: false, delete not refused).collectionParts, withTestFleetGatheredStagedMemberPreviewsItsStagedContent. It fails with the revision check removed, because the preview shows only the live provider node.system-go/fleet_script_network_test.go. A recordinghttp.dohost asserts zero requests over fleet nodes for the serve-path render, the staged render core runs for a plan, a fleet-bound collection's own chain, and a provider member's chain inside one. Separate tests assert that the editor's preview shows the typed error, thatprocessNodeswithholds the network on its own, and that the script sees no_latticeblock, placeholder, line uuid, line hash id, node id or row address while the plan still binds every node to its line. That last test runs one line per bind-shape protocol (vless, trojan, anytls, hysteria2, tuic, vmess, socks) and also fails with the placeholder masking alone removed. A provider-render control keeps its request. With the fix's two mechanisms reverted (the original node encoding inprocessNodes, andwithholdScriptHTTPinstalling nothing), seven of the eight fail. Each render sends one request through the host, and the script reads the Lattice block.TestFleetScriptStepRunsOnBundleProcessandTestInventedNodeInProviderCollectionIsNoLinewere adapted: the first now forges a_latticeblock outright, and the second finds a fleet node as the one without a provider mark.TestProxyMethodsDeclareVpncoreReadfails with the manifest change reverted and passes with it. The manifest tests pass, andlattice-plugin-manifest-checkat the CI pin accepts the manifest.go test ./...insystem-go,-raceon the touched tests (noDATA RACE), and the UIvitest runandvue-tscall pass.lane/20261016_s2-gatesat 3eb17e1). There the plugin suite passes,TestCredentialAbsenceSweepincluded. Lane 7's server (lattice-server42853c1) was pinned at that merge, and its 17 S2 end-to-end tests all pass with no skips,TestScriptOperatorForeignServerIsPlanRejectedamong them. A throwaway end-to-end check on that server confirmed the core path. A provider member edit stages,plans.publishrefuses it, propose requires a plan whose dependent section shows theprovider-contentchange, and approval serves it. A direct delete of a member gathered by tag answersfleet_delete_requires_plan, and the delete plan removes it.Not done
plans.proposeanswersbind preview failed for revision ... and identity ...andbind.previewanswersthe record did not render, both checked on 42853c1.ui/dev/fleetHost.tsfleetWithhelddoes not yet dropmigrate_recordfor?fleet=denied. Left for lane 6b, whose migration flow is the consumer.Contract for core and the UI
previewandpreview_drafterrors for this case lead withfleet_script_network_unavailable:, meaning the code, a colon, then a space, the same shape as the other S2 read-method codes. Everything after the colon is plugin text, which names the record and a request count and never what the script sent. For the plan view, core can lift a leading code that matches a fixed allowlist into the propose andbind.previewerror, and relay nothing else of the plugin's message. No new field is added to the render reply or the plan.staged: trueon save andfleet_delete_requires_planon delete although its list flags show it is not fleet-bound. The UI should offer the plan flow from the reply, not from the flag. Core needs no change for this:depends_onalready reports the dependents.vpncore:read, a token no longer seesmigrate_recordin the handshake.