feat(proto,sandbox): SandboxExecService + RemoteRunner client (infra-admin P3b PR7/12)#847
Merged
Merged
Conversation
…agent) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ine→agent)
Implements sandbox.SandboxRunner via streaming gRPC to a remote sandbox agent.
mTLS + bearer token auth. env values passed verbatim (secret:// unresolved, ADR 0017).
Tested with in-process bufconn stub; mTLS wired end-to-end in PR11 scenario.
Hardening (code review):
- Exec returns an error when the stream closes without an exit_code chunk
(agent crash / truncated stream) instead of a silent ExecResult{ExitCode:0}.
- connect() returns the *grpc.ClientConn so Exec holds it locally; a concurrent
Close() (conn=nil under mu) can no longer nil-deref / race an in-flight Exec.
- NewRemoteRunner rejects Token over a non-TLS connection unless AllowInsecure
is set, preventing cleartext bearer-token leaks.
- .golangci.yml: single \.pb\.go$ path exclusion for generated code (gosec G103
+ staticcheck QF1008; gosec ignores the golangci generated: setting).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the typed gRPC contract and a Go client implementation for dispatching sandboxed command execution to a remote agent (infra-admin Phase 3b, PR7/12), as groundwork for the upcoming agent binary/config wiring in PR8.
Changes:
- Introduces
SandboxExecServiceprotobuf contract (Execstreaming stdout/stderr chunks + terminalexit_code). - Adds
sandbox/remote.RemoteRunnerimplementingsandbox.SandboxRunnerover gRPC with optional mTLS + bearer token auth and connection caching. - Updates golangci-lint config to exclude protobuf-generated
*.pb.gofiles from linting.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sandbox/remote/runner.go | Implements RemoteRunner gRPC client, streaming exec and connection lifecycle. |
| sandbox/remote/runner_test.go | Adds unit tests for streaming behavior, secret ref pass-through, concurrency, and auth guardrails. |
| plugin/external/proto/sandbox_exec.proto | Defines typed proto contract for remote sandbox execution. |
| plugin/external/proto/sandbox_exec.pb.go | Generated protobuf Go types for sandbox exec contract. |
| plugin/external/proto/sandbox_exec_grpc.pb.go | Generated gRPC client/server bindings for sandbox exec contract. |
| .golangci.yml | Excludes *.pb.go from golangci-lint to avoid non-actionable findings in generated code. |
Files not reviewed (2)
- plugin/external/proto/sandbox_exec.pb.go: Language not supported
- plugin/external/proto/sandbox_exec_grpc.pb.go: Language not supported
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
- Exec stops reading as soon as the terminal exit_code chunk arrives (was waiting for io.EOF → could hang if the agent sends exit_code but doesn't close the stream). - Exec rejects an empty argv before any RPC, matching the local DockerSandbox runner. Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
PR7/12 — SandboxExecService proto + RemoteRunner client (infra-admin Phase 3b, part 1)
Locked plan Tasks 13–14; ADR 0019. The typed gRPC contract + client for dispatching a sandboxed command to a remote agent. The agent binary + engine config wiring land in PR8.
Changes
plugin/external/proto/sandbox_exec.proto(co-located in the existing buf module — the editor/toolchain home for all repo protos; design backport vs the plan's tentativesandbox/remote/proto):SandboxExecService { rpc Exec(SandboxExecRequest) returns (stream SandboxExecChunk); }. Typed messages (no Struct/Any).envcarries UNRESOLVEDsecret://refs (agent resolves them, ADR 0017);profileis the requested profile (agent clamps it, PR8). Generated viabuf generate(touched only the new files).sandbox/remote.RemoteRunnerimplementssandbox.SandboxRunner: dials over mTLS + bearer token, streamsExec, accumulates stdout/stderr + the exit code, returns*sandbox.ExecResult. Conn cached;Close()idempotent. Does NOT resolvesecret://refs (ADR 0017).Review notes (resolved)
ExitCode:0success.connect()returns the conn (held locally) so a concurrentClose()can't nil-race it;-racetests added (16 concurrent Exec; Exec racing Close).NewRemoteRunnerrefuses a non-emptyTokenover a nil-TLS connection unlessAllowInsecureis explicitly set (no accidental cleartext-token leak).Verified:
go build ./...exit 0;go test -race ./sandbox/remote/pass; fullgo test ./...exit 0 (150 ok); full-repogolangci-lint v2.12.00 issues (generated.pb.goexcluded —exclusions.generated: laxdoesn't cover gosec G103 on protoc output, so a single\.pb\.go$path exclusion is used).🤖 Generated with Claude Code