feat: support golang - #41
Conversation
A daemon proves who it is with a signed token, presented as a bearer Authorization header on the WebSocket upgrade. Until now any process that could reach the controller could register as any daemon id, which is the whole tenant boundary when one controller serves a fleet: registering as someone else's id redirects their exec traffic. server/src/auth.rs verifies with the PUBLIC half only, so the controller holds nothing worth stealing — the private key never leaves whoever mints tokens (Nebula's manager). Checked: signature, `iss`, `aud`, `exp`, `kid` selection, and a non-empty `sub`. Verification failures log the detail for the operator but answer a generic 401, so a prober cannot learn which part of its forgery to fix. Authentication happens BEFORE the upgrade. An unauthenticated caller never gets a socket, cannot hold server resources or reach the message loop, and gets a plain HTTP 401 it can actually understand rather than a close frame after a successful handshake. registry.rs gains the stats surface the /stats endpoint and the FFI host read (per-daemon hostname/platform/labels/heartbeat age), keyed by daemon id with the id not repeated inside the value. Signed-off-by: kerthcet <kerthcet@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: kerthcet <kerthcet@gmail.com>
The controller had exactly one entry point: a pyo3 extension module. That made it awkward as infrastructure — running it meant paying for a CPython interpreter, the extension module and a second event-loop owner to host a process that never executes a line of Python. Two entry points are added, both re-exposing the same registry, protocol and token verification rather than reimplementing them. src/main.rs is the `sandd-controller` binary: a real argv/env config surface, no interpreter, ~10x smaller image. Auth is opt-in but not silently downgradable — --enable-auth with missing material is an error, never a quiet fallback to accepting everyone. src/ffi.rs is a C ABI for hosts that are not CPython. Nebula's Go manager links it via cgo to run the controller IN-PROCESS, which is what lets its virtual kubelet reach back into a workload for `kubectl exec` instead of asking a second process to relay a live socket. go/controller wraps it. Making this work needed the pyo3 dependency to become OPTIONAL, behind a `python` feature. With `extension-module` set, pyo3 deliberately leaves the CPython symbols undefined for an interpreter to supply at dlopen time, so any plain `cargo build` of a bin target fails to link — loudly on macOS, subtly elsewhere. The crate now builds as staticlib too, so a cgo host can link an archive and stay a single self-contained binary on a static base image instead of shipping a .so beside it. The Python bindings are unaffected: maturin builds with --features python (pyproject.toml), which is now required or the wheel would contain no `_core` at all. Signed-off-by: kerthcet <kerthcet@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: kerthcet <kerthcet@gmail.com>
An authenticated connection now registers under its token's `sub`, and the id in the Register message is ignored. The token is the whole identity. The first version of this compared the two and rejected a mismatch, which was strictly worse. The daemon falls back to a random UUID when it is not told an id, so a host that delivered a token but no SANDD_DAEMON_ID did not get an unnamed-but-working daemon: it connected, was refused, and never entered the registry — indistinguishable downstream from "no daemon yet", with nothing pointing at the cause. Nebula's AWS bootstrap shipped with exactly that bug. Deriving the id here means no host can get it wrong, because there is only one place the identity comes from. Unauthenticated mode is unchanged: the claimed id is used (an empty one is refused, since nothing could address that entry). That path is not vestigial — the e2e compose file, the READMEs and the Python integration tests all run daemons with --daemon-id against a controller with no auth. The daemon side gains SANDD_TOKEN, sent as a bearer header on the upgrade. Env-only with no CLI flag on purpose: an argument is world-readable through /proc/<pid>/cmdline, so any process on the instance — including the workload the daemon runs beside — could `ps` the token out and impersonate it. The header is marked sensitive so it stays out of any Debug output. SERVER_URL/DAEMON_ID also become SANDD_*, since the daemon runs inside the user's own image where an unnamespaced name can collide. Signed-off-by: kerthcet <kerthcet@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: kerthcet <kerthcet@gmail.com>
The controller stopped being a deployed artifact when Nebula started linking it into its own manager process through the C ABI (server/src/ffi.rs). A daemon's connection is a live socket owned by whichever process accepted it, so reaching a workload from a separate controller process would mean relaying. That left the release publishing two things nothing consumes: the sandd-controller binaries and the inftyai/sandd-controller image. Publishing them implies a supported deployment shape that has no users, so drop both matrix legs and the controller-image job. The daemon assets are untouched. `cargo build --bin sandd-controller` and `make docker-build-controller` still work for running it standalone. Also fix a Makefile comment that pointed at internal/controller/pod_placement_controller.go, a path whose SandD code was deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: kerthcet <kerthcet@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a first-class “controller” surface intended to be embedded/consumed from Go (via a Rust C ABI + Go wrapper), while also introducing daemon authentication on /ws (JWT/Ed25519 verification) and expanding controller observability (/stats includes per-daemon detail). It reorganizes the sandbox-server crate so the same Rust implementation can be built as: a native controller binary, a Python extension (feature-gated), and an FFI/staticlib for Go.
Changes:
- Add daemon token authentication on WebSocket upgrade (
/ws) with an auth-enabled constructor and shared app state. - Add a Rust C ABI (
server/src/ffi.rs) + Go wrapper module (go/controller) for embedding the controller in Go. - Expand registry stats to include per-daemon details and expose them via
/stats(and FFI JSON), plus build/docs updates for controller artifacts.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/server.rs | Adds optional auth gating for /ws, shared AppState, and richer /stats response wiring. |
| server/src/registry.rs | Extends RegistryStats with per-daemon detail (DaemonInfo). |
| server/src/python.rs | Moves Python bindings behind a python feature into a dedicated module. |
| server/src/main.rs | Introduces the sandd-controller binary with clap/env-based configuration and auth wiring. |
| server/src/lib.rs | Reorganizes crate modules for multi-consumer layout (bin, python feature, ffi feature). |
| server/src/ffi.rs | Adds C ABI to drive controller/registry/exec/session from non-Python hosts (e.g., Go). |
| server/src/auth.rs | Adds Ed25519 JWT verification (TokenVerifier), bearer parsing, and tests. |
| server/Cargo.toml | Adds sandd-controller bin, ffi/python feature gating, and staticlib output. |
| sandd/src/main.rs | Adds optional daemon bearer token header on upgrade; env var naming updates. |
| pyproject.toml | Ensures maturin builds the Python layer via the new python feature. |
| Makefile | Adds controller build targets and multi-arch Docker targets/documentation for images. |
| hack/docker/README.md | Updates tunnel image build guidance to use multi-arch Makefile targets. |
| hack/docker/Dockerfile.controller | Adds a distroless-based controller image build for the native binary. |
| go/go.mod | Introduces a Go module for controller bindings. |
| go/controller/controller.go | Implements a safe-ish Go wrapper over the Rust C ABI. |
| go/controller/controller_test.go | Adds Go-level tests for lifecycle, misconfig, and concurrency behavior. |
| examples/tunnel-simple/README.md | Updates local build instructions to use the new Make targets. |
| docs/proposals/TUNNEL.md | Updates tunnel image build instructions for multi-arch Make targets. |
| Cargo.lock | Updates lockfile for new dependencies (e.g., clap, jsonwebtoken). |
| .github/workflows/release.yaml | Refactors release workflow steps and checksum generation; clarifies daemon-only release artifacts. |
| .dockerignore | Stops excluding examples/ to keep workspace manifests parseable in Docker builds. |
Suppressed comments (1)
go/controller/controller.go:533
Session.Closecan free the underlyingSanddSession*while another goroutine is insideRead/Write/Resize. In particular,Readcopiess.ptrunders.muand then calls into C without holdings.mu, so a concurrentClosecansandd_session_freethe same pointer, leading to use-after-free / undefined behavior.
func (s *Session) Close() error {
s.mu.Lock()
if s.ptr == nil {
s.mu.Unlock()
return nil
}
ptr := s.ptr
s.ptr = nil
s.mu.Unlock()
if s.srv != nil {
s.srv.forget(s)
}
C.sandd_session_free(ptr)
return nil
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Exec and Session.Read must not hold the mutex while they park in C — otherwise one idle terminal serializes every other caller — so they copy the handle and release the lock. Nil'ing the pointer on Close is then not enough to make the free safe: a blocked call still holds its own copy. On the server that is worse than a stale-handle read. sandd_server_free drops the tokio Runtime the struct owns, and sandd_exec holds `srv.runtime.block_on(...)` across its whole wait, so a concurrent Close frees a runtime a thread is currently executing on. Count in-flight calls on both types and have Close drain that count before freeing. Acquiring checks the pointer and increments under the mutex, so a caller arriving after Close is refused with ErrClosed and cannot join the set being waited on. Server.Close closes its sessions first, as before, then waits — a session's read parks on a handle to the server's runtime, which is what makes waiting there sufficient. Close can now block for as long as the longest outstanding timeout. That is bounded, and the alternative is freeing a live executor. Nebula's relay is unaffected: it closes from a defer holding no lock, and polls reads at 500ms rather than parking for the session's lifetime. Session.Close's comment claimed a parked reader was safe because it "holds its own pointer copy". That is exactly what made it unsafe; the comment is corrected along with the package-level lifetime docs. Both tests fail with the two waits removed and pass with them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: kerthcet <kerthcet@gmail.com>
|
/lgtm |
InftyAI-Agent
left a comment
There was a problem hiding this comment.
Approved: PR has both lgtm and approved labels
InftyAI-Agent
left a comment
There was a problem hiding this comment.
Approved: PR has both lgtm and approved labels
What this PR does / why we need it
Which issue(s) this PR fixes
Fixes #
Special notes for your reviewer
Does this PR introduce a user-facing change?