Skip to content

feat(egress): fastlet Sandbox Actions handler protocol for fleet profile (OSEP-0022) - #1678

Open
Pangjiping wants to merge 5 commits into
opensandbox-group:mainfrom
Pangjiping:feat/egress-actions-handler
Open

feat(egress): fastlet Sandbox Actions handler protocol for fleet profile (OSEP-0022)#1678
Pangjiping wants to merge 5 commits into
opensandbox-group:mainfrom
Pangjiping:feat/egress-actions-handler

Conversation

@Pangjiping

@Pangjiping Pangjiping commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Background

The fleet profile's subject lifecycle switches from the fast-sandbox slot-store file observation (/run/fast-sandbox/network/*.json) to the public Sandbox Actions Handler protocol (sandbox.fast.io/actions/v1): the Fastlet delivers SET_BINDING / LIFECYCLE_HOOK / REMOVE_BINDING over Pod-loopback HTTP and the egress process is the Handler. The file-driven approach is removed entirely.

Changes

New

  • pkg/actionhandler: action envelope wire model, parsing and validation (apiVersion/operation/Hook allowlists, null-input vs ordinary string inputs, required fencing fields)
  • fleet_actions.go: GET /_fastlet/v1/actions/status (per-process instanceId, triggers Fastlet replay after restart) + POST /_fastlet/v1/actions

Operation mapping

  • SET_BINDING: registers the subject deny-first (attachment provides IP/gateway/veth/CIDR), stores the policy pending; updates on an already-active subject apply in place; null input (binding removed) reverts to deny-first
  • LIFECYCLE_HOOK: sandbox.runtime-ready confirms; sandbox.data-plane-ready applies the policy -> active; no pending policy fails closed (409)
  • REMOVE_BINDING: terminal cleanup; stale removals (fence mismatch) are ignored; missing state is success

Removed (-3133 lines)

  • pkg/slotsource (file observation), pkg/subject/controller.go (file-driven lifecycle), pkg/sandboxnft (netns OUTPUT defense-in-depth layer; the envelope has no netnsPath), pkg/resolvrewrite
  • OPENSANDBOX_EGRESS_SLOT_STORE_DIR / _SLOT_POLL_INTERVAL env vars, fleetnft.ApplyDispatchUpdate

Preserved semantics

  • Fail-closed: deny-first stays in effect from SET_BINDING until data-plane-ready
  • Credentials still flow over the proxy route /credential-vault (memory-only, OSEP-0012; binding input is persisted in the Sandbox CRD and is not a secret transport), pending cache fenced on specGeneration
  • Shared DNS proxy / shared MITM / connection refresh / ApplyReset restart recovery unchanged

Docs

  • OSEP-0022 updated to the API-based integration (channel 1 = actions protocol, channel 2 = proxy route credentials); source line numbers and code sketches removed

Notes

  • The go.mod/vendor inconsistency is pre-existing in the repo (not introduced by this PR); verified locally with -mod=mod
  • fast-sandbox side needs: Pool actionHandlers declaring egress (targetHTTPPort 18080, hooks: runtime-ready + data-plane-ready); the four internal additions for the credential channel are detailed in OSEP-0022

@github-actions github-actions Bot added component/egress documentation Improvements or additions to documentation size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 31, 2026

@jianpingpei jianpingpei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] The fleet DNS interception does not match the resolver that FastSandbox currently installs. FastSandbox LinuxNetNSDriver copies the Fastlet resolver file into each slot (current test/example value is 10.96.0.10), while SetupGatewayDNSRedirect only redirects port 53 when the destination is attachment.network.gateway. Therefore real sandbox DNS traffic does not reach this proxy: default-deny can make allowed domains unresolvable, while default-allow can bypass domain deny and DNS-based IP learning. The fleet smoke queries 10.10.0.1 directly and the Firecracker E2E writes the gateway into resolv.conf, so they do not exercise the real integration. Please either make FastSandbox provide the gateway as the slot resolver when this handler is enabled, or intercept sandbox-veth DNS independently of the resolver destination, and cover it with an actual FastSandbox integration test.

Comment thread components/egress/fleet_actions.go Outdated
Comment thread components/egress/fleet_actions.go Outdated
Comment thread components/egress/fleet_actions.go
Comment thread components/egress/fleet_actions.go Outdated
Comment thread components/egress/fleet_actions.go
Pangjiping added a commit to Pangjiping/OpenSandbox that referenced this pull request Aug 31, 2026
…ndbox-group#1678)

Addresses the four P1s and the P2 from the review:

- /policy pushes for a StateDenying subject are stored as the pending policy
  instead of applying immediately: the lifecycle barrier (data-plane-ready)
  is the only activation signal. Cached pushes flushed at SET_BINDING now
  land AFTER the binding input's pending store, so the newer pushed policy
  wins and still waits for the Hook.
- data-plane-ready peeks the pending policy instead of consuming it: a
  transient nft failure keeps it in place, so the Fastlet's retry of the
  same Hook succeeds instead of 409-ing forever.
- LIFECYCLE_HOOK is fenced against the registered identity: a delayed Hook
  from a previous instance can no longer consume the replacement sandbox's
  pending policy or activate it before its own data plane is ready.
- REMOVE_BINDING keeps the subject registered until terminal enforcement
  cleanup succeeds: a retried removal resumes cleanup instead of returning
  success with stale kernel rules. OnUnloaded removes nft first so a
  failure leaves every teardown step undone (no double gateway release).
- gateway DNS redirect ownership is keyed per subject (not refcounted per
  gateway): duplicate SET_BINDING delivery is a no-op, a gateway move on
  rebind releases the old gateway, and one unload fully releases it.

New tests: push-while-denying barrier, data-plane-ready retry after nft
failure, stale-Hook fence rejection, REMOVE_BINDING retry after nft failure,
duplicate-registration gateway idempotency, and gateway-move rebind.
…ile (OSEP-0022)

The fleet profile's subject lifecycle switches from the fast-sandbox slot-store
file observation (/run/fast-sandbox/network/*.json) to the public Sandbox
Actions Handler protocol (sandbox.fast.io/actions/v1): the Fastlet delivers
SET_BINDING / LIFECYCLE_HOOK / REMOVE_BINDING over Pod-loopback HTTP and the
egress process is the Handler.

- pkg/actionhandler: action envelope wire model, parsing and validation
  (apiVersion/operation/Hook checks, null-input vs ordinary string inputs)
- fleet_actions.go: /_fastlet/v1/actions/status (instanceId incarnation probe)
  and /_fastlet/v1/actions; SET_BINDING registers deny-first and stores the
  policy pending, sandbox.data-plane-ready activates it, null input reverts
  to deny-first, REMOVE_BINDING cleans up (stale fence ignored)
- subject: Fencing from revision.runtimeInstanceId/attachmentId, lifecycle
  hooks take the attachment network block, registry gains UnsetPolicy/Fence
- fleetnft: attachment-based dispatch, ApplyDispatchUpdate removed
- deleted: pkg/slotsource, pkg/subject/controller (file-driven lifecycle),
  pkg/sandboxnft (netns OUTPUT layer), pkg/resolvrewrite
- proxy route /policy + /credential-vault unchanged (credentials stay
  memory-only, OSEP-0012; pending-push cache fenced on specGeneration)
- OSEP-0022 updated: API-based integration, credential channel on the proxy
  route; docs updated

docs(oseps): OSEP-0022 status draft -> implementing
…ions protocol

The fleet profile no longer reads the slot store, so both test scripts
register subjects through SET_BINDING / LIFECYCLE_HOOK / REMOVE_BINDING
instead of writing slot files:

- smoke-fleet.sh: set_binding/lifecycle_hook/remove_binding helpers posting
  action envelopes to /_fastlet/v1/actions; deny-first asserted at SET_BINDING,
  activation at data-plane-ready; rebind via a new identity fence; restart
  recovery via instanceId replay; dropped the resolv.conf and per-sandbox
  netns OUTPUT assertions (layers removed); MITM phase re-registers through
  actions
- e2e-fleet-firecracker.sh: bind_subject/activate_subject replace write_slot;
  policy rides the binding input; removed the slot-store/resolv volumes and
  env from both containers
- firecracker e2e doc: contract updated to the actions protocol

fix(egress): encode binding input as a JSON string in the smoke/e2e actions envelopes

fix(egress): inline curl in wait_for checks (shell functions are not visible in bash -c)
…ndbox-group#1678)

Addresses the four P1s and the P2 from the review:

- /policy pushes for a StateDenying subject are stored as the pending policy
  instead of applying immediately: the lifecycle barrier (data-plane-ready)
  is the only activation signal. Cached pushes flushed at SET_BINDING now
  land AFTER the binding input's pending store, so the newer pushed policy
  wins and still waits for the Hook.
- data-plane-ready peeks the pending policy instead of consuming it: a
  transient nft failure keeps it in place, so the Fastlet's retry of the
  same Hook succeeds instead of 409-ing forever.
- LIFECYCLE_HOOK is fenced against the registered identity: a delayed Hook
  from a previous instance can no longer consume the replacement sandbox's
  pending policy or activate it before its own data plane is ready.
- REMOVE_BINDING keeps the subject registered until terminal enforcement
  cleanup succeeds: a retried removal resumes cleanup instead of returning
  success with stale kernel rules. OnUnloaded removes nft first so a
  failure leaves every teardown step undone (no double gateway release).
- gateway DNS redirect ownership is keyed per subject (not refcounted per
  gateway): duplicate SET_BINDING delivery is a no-op, a gateway move on
  rebind releases the old gateway, and one unload fully releases it.

New tests: push-while-denying barrier, data-plane-ready retry after nft
failure, stale-Hook fence rejection, REMOVE_BINDING retry after nft failure,
duplicate-registration gateway idempotency, and gateway-move rebind.
@Pangjiping
Pangjiping force-pushed the feat/egress-actions-handler branch 2 times, most recently from 19c78d9 to 460b1cb Compare August 31, 2026 13:01
@Pangjiping
Pangjiping marked this pull request as ready for review August 31, 2026 13:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 460b1cb324

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

// window is newer intent than the binding input, so it overwrites the
// pending policy (the lifecycle barrier still holds — it only becomes
// effective at data-plane-ready). Vault pushes apply regardless.
s.OnRegisteredComplete(subj, att, env.Revision.SpecGeneration)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Defer pending vault replay until policy activation

When a credential-vault push with host bindings arrives before SET_BINDING, it is cached with 202, but this call replays it while the subject is still denying and EffectivePolicy is nil. credentialvault.Store.Create consequently rejects the bindings as requiring an egress policy; takePendingAll has already removed the request, and the failure is only logged, so the server believes the credential revision was accepted while the sandbox never receives it. Retain vault pushes until data-plane-ready or validate them against the pending binding policy.

AGENTS.md reference: AGENTS.md:L44-L44

Useful? React with 👍 / 👎.

Comment on lines +219 to +224
pol := s.pendingPolicy(subj)
if pol == nil {
// No pending policy: SET_BINDING has not landed (protocol
// ordering violation) or the binding was removed. Fail closed —
// the subject must never activate without its current policy.
return http.StatusConflict, fmt.Errorf("data-plane-ready for subject %s with no pending policy", subj)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make repeated data-plane-ready hooks idempotent

Under the documented at-least-once delivery model, if the first successful response is lost, the Fastlet retries the same data-plane-ready invocation after the policy has become active and the pending entry was cleared. This branch then returns 409 indefinitely, so the Fastlet cannot acknowledge the hook and may keep the binding or sandbox unready despite successful enforcement. Return success when the matching subject is already active, or deduplicate by invocationId.

AGENTS.md reference: AGENTS.md:L119-L120

Useful? React with 👍 / 👎.

Comment on lines +769 to 770
if err := s.nft.Remove(nftCtx, subj); err != nil {
return err

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve nft state when removal transactions fail

If the nft rebuild fails during REMOVE_BINDING, fleetnft.Applier.Remove has already deleted the subject from its in-memory subjects map even though the atomic transaction left the old kernel table live. This call returns 500 as intended, but the Fastlet's retry sees the subject as absent in the applier, returns success, and unregisters it here while its old allow rules remain installed. The applier must commit its map deletion only after the nft transaction succeeds, or restore the entry on failure.

Useful? React with 👍 / 👎.

Comment on lines +166 to +168
if state == subject.StateActive {
if err := s.revertToDenyFirst(subj, att); err != nil {
return http.StatusInternalServerError, err

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reuse the stored attachment for null binding updates

The protocol validator explicitly permits SET_BINDING with input: null to omit attachment, but this path passes env.Network()—an empty attachment—to revertToDenyFirst. For an active subject, the resulting nft script contains an invalid IP/veth and the supported policy-removal operation returns 500; for a denying or newly replayed subject, registration fails similarly. Resolve the current attachment from the server's stored binding state instead of requiring an optional field.

AGENTS.md reference: AGENTS.md:L119-L120

Useful? React with 👍 / 👎.

…ndbox-group#1699)

With net.bridge.bridge-nf-call-iptables=1 (the fast-sandbox Firecracker
bridge topology), an explicit accept verdict from the forward hook returns
the frame to the bridge L2 path: a frame whose destination is the bridge
itself is then treated as local delivery and dropped before postrouting, so
IP-direct egress for allowed subjects never reaches SNAT. Only 'not hitting
a drop rule' lets the frame continue IP routing.

The forward path no longer accepts explicitly:
- new per-subject 'hook prerouting' mark chains (shared mark hook chain +
  per-subject jump) set meta mark 0x2 for allow/dyn set members
  (default-deny) or unconditionally (default-allow)
- the master dispatch chain flips to policy accept with an unmarked-drop
  tail (meta mark & 0x2 != 0x2 drop): unregistered sources and deny-first
  subjects carry no mark and are denied by the tail (fail-closed preserved)
- per-subject forward chains keep only the deny-set drops (no accept
  verdicts); the INPUT (MITM) chain keeps its full verdicts unchanged
- deny-first resets flush the mark chain; policy swaps rebuild it (a
  default-action change flips the mark strategy)
- mark 0x2 is distinct from the DNS proxy's SO_MARK 0x1 bypass

Unit tests: mark-based shapes (default-deny set marks, default-allow
unconditional mark, deny-first resets, no forward accepts), v6 mark jump.
@Pangjiping
Pangjiping force-pushed the feat/egress-actions-handler branch from f5f3ed4 to 85ea85b Compare September 1, 2026 16:43
…word

The chain name 'mark' is rejected by nftables ('syntax error, unexpected
mark, expecting string or last') because 'mark' is a reserved keyword (meta
mark). Rename the shared prerouting hook chain to 'marking'; the per-subject
mark_<id> chains are unaffected. Verified the full generated script
(reset + deny-first + policy swap, DoH + MITM) against nftables 1.0.x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/egress documentation Improvements or additions to documentation size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants