Skip to content

ci: add GitHub Actions CI workflow with lint, test, and image build - #1

Merged
drew merged 15 commits into
mainfrom
dn/ci-tests
Feb 27, 2026
Merged

ci: add GitHub Actions CI workflow with lint, test, and image build#1
drew merged 15 commits into
mainfrom
dn/ci-tests

Conversation

@drew

@drewdrew commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds .github/workflows/ci.yml with lint, rust-test, python-test, and CI image build jobs
  • Adds build/scripts/ci-image.sh for local + CI docker image builds
  • Adds docker:buildx:setup mise task for multi-arch remote BuildKit configuration
  • Updates docker:build:ci mise task to use the new script

@drew
drewforce-pushed the dn/ci-tests branch 3 times, most recently from 2698555 to bda31e5CompareFebruary 27, 2026 07:42
…action
Use docker/setup-buildx-action@v3 wrapped in a local composite action
to create the multi-arch remote BuildKit builder. This ensures automatic
cleanup of builder nodes when the job finishes, preventing leaked
BuildKit instances on self-hosted runners.
Declare the ARG before mise install so authenticated GitHub API
requests avoid rate limits when fetching tool versions.
@drew
drew merged commit 7e7e84e into mainFeb 27, 2026
3 checks passed
@drew
drew deleted the dn/ci-tests branch February 27, 2026 18:10
TylerBarkley added a commit to TylerBarkley/OpenShell that referenced this pull request Mar 24, 2026
Implement the final integration for GPU passthrough support:
- Add put_vfio_device() API function for PUT /vfio/{device_id}
- Wire prepare_gpu_attachment() into do_create_inner()
- Enable supports_gpu in Firecracker capabilities
- Update spec compatibility checks and tests
This completes blockers NVIDIA#1 (GPU attachment not wired) and NVIDIA#2
(VFIO device attachment API not implemented).
Signed-off-by: OpenCode Agent <opencode@nvidia.com>
@mjamivmjamiv mentioned this pull request Apr 6, 2026
2 tasks
ben-alkov pushed a commit to ben-alkov/OpenShell that referenced this pull request Apr 13, 2026
feat(podman): add Podman support as alternative container runtime
ben-alkov pushed a commit to ben-alkov/OpenShell that referenced this pull request Apr 13, 2026
docs(podman): Add macOS installation guide and Podman build compatibility
mrunalp added a commit to mrunalp/OpenShell that referenced this pull request Apr 18, 2026
1. Separate client_id from audience (Critical/High, findings NVIDIA#1/NVIDIA#3):
- Add oidc_audience field to GatewayMetadata separate from oidc_client_id
- Bootstrap stores client_id and audience independently
- Fixes the conflation that made the docs overstate provider portability
2. Skip list risk documentation (Critical, finding NVIDIA#2):
- The skip list is by design for sandbox supervisor RPCs which use
SSH handshake secrets. Noted for future hardening with per-sandbox
credentials.
3. Reject partial-empty RBAC config (Medium, finding NVIDIA#4):
- AuthzPolicy::validate() rejects configs where only one of
admin_role/user_role is set
- Server validates at startup before accepting requests
- Prevents silently opening admin endpoints to any authenticated user
4. Preserve refresh token on refresh (Medium, finding NVIDIA#5):
- oidc_refresh_token() keeps the old refresh_token when the server
doesn't return a new one, per OAuth 2.0 spec
5. Additional concerns:
- Percent-decode callback query parameters (code, state, error)
- Drop scope=openid from client_credentials flow
- Use /dev/urandom for PKCE verifier/state on Unix
- Validate discovery issuer matches configured issuer (both server
and CLI) to prevent SSRF/misdirection
- Wire RBAC config (rolesClaim, adminRole, userRole) through Helm
values and statefulset template
mrunalp added a commit to mrunalp/OpenShell that referenced this pull request Apr 18, 2026
1. Critical: sandbox RPCs now require x-sandbox-secret header (NVIDIA#1):
- Split skip list into unauthenticated (health/reflection) and
sandbox-secret (supervisor RPCs) categories
- Sandbox RPCs require x-sandbox-secret header matching the server's
SSH handshake secret
- Sandbox gRPC client injects the secret via SandboxSecretInterceptor
- Unauthenticated network callers can no longer reach sandbox RPCs
2. High: complete client_id vs audience separation (NVIDIA#2):
- Add --oidc-audience flag to gateway add (separate from --oidc-client-id)
- Store oidc_audience in gateway metadata independently
- Thread audience through bootstrap metadata correctly
3. Medium: propagate RBAC settings through bootstrap (NVIDIA#3):
- Add oidc_roles_claim, oidc_admin_role, oidc_user_role to DeployOptions
- Pass OIDC_ROLES_CLAIM, OIDC_ADMIN_ROLE, OIDC_USER_ROLE env vars
through Docker container → cluster-entrypoint.sh → HelmChart manifest
- Full provider-neutral RBAC config now works via gateway start
4. Low: cross-platform CSPRNG (NVIDIA#4):
- Replace /dev/urandom + RandomState fallback with getrandom crate
- Works correctly on all platforms (Linux, macOS, Windows)
mrunalp added a commit to mrunalp/OpenShell that referenced this pull request Apr 18, 2026
1. High: UpdateConfig dual-auth for sandbox policy sync (NVIDIA#1):
- Add DUAL_AUTH_METHODS list for RPCs that accept either Bearer
token (CLI users) or sandbox secret (supervisor)
- UpdateConfig now works for both CLI policy mutations and
sandbox startup policy sync in OIDC deployments
2. Medium: end-to-end client_id vs audience separation (NVIDIA#2):
- Thread optional audience parameter through oidc_browser_auth_flow
and oidc_client_credentials_flow
- gateway login reads oidc_audience from metadata and passes it
to the auth/token requests
- Adds audience parameter to authorization URL for providers
that require it (Entra ID)
3. Medium: gateway start exposes full OIDC RBAC config (NVIDIA#3):
- Add --oidc-client-id, --oidc-roles-claim, --oidc-admin-role,
--oidc-user-role flags to gateway start
- Thread all values through gateway_admin_deploy into DeployOptions
- Stop hardcoding "openshell-cli" as client_id in bootstrap metadata
zredlined added a commit that referenced this pull request May 12, 2026
…drafts
Three things in one commit, all surfaced by running the demo end-to-end
against a real gateway and finding the agent had to draft a broader
second proposal.
1. /wait race fix. Previously /wait returned `approved` the moment it
observed the gateway's chunk status flip, but the local supervisor
reloads policy on its own poll cycle (~10s in practice). The agent's
retry would race the reload and hit the still-old policy, getting
denied. Codex then drafted a broader rule and re-submitted — sound
agent behavior, but not what /wait should provoke. Now /wait captures
the local policy version at start, and after observed-approved waits
for the supervisor to load a strictly-newer version before returning.
Bounded by the caller's deadline; best-effort return if the deadline
elapses without the version bumping. Two new unit tests pin the
happy path and the deadline-clamped fallback.
2. demo.sh auto-approve loop. Replaces approve_when_pending +
wait_for_agent with one approve_pending_until_agent_exits function
that keeps watching for pending chunks and approving them until the
agent process exits (or the configured timeout). Defense in depth
against future redraft scenarios for any reason; today (post-fix#1)
the agent should only submit one proposal per task, but we don't
want to hang silently if it does submit more.
3. UX. Step headers now carry "[t+1.2s]" relative timestamps so reading
the run output makes latency visible (the demo's whole point is the
wait is cheap — surface that). A spin_wait helper renders an ASCII
spinner during the watch loop so the demo never looks frozen on a
TTY. Falls back to plain sleep on non-TTY contexts.
Closes the race condition diagnosed from the trace timing where the
gateway approved at t+0, sandbox observed at t+0.3s, but the supervisor
didn't load v2 until t+9.4s — well after the agent had already retried
and been denied.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
@johntmyers

Copy link
Copy Markdown
Collaborator

smoke test - ignore

zredlined added a commit that referenced this pull request May 13, 2026
…drafts
Three things in one commit, all surfaced by running the demo end-to-end
against a real gateway and finding the agent had to draft a broader
second proposal.
1. /wait race fix. Previously /wait returned `approved` the moment it
observed the gateway's chunk status flip, but the local supervisor
reloads policy on its own poll cycle (~10s in practice). The agent's
retry would race the reload and hit the still-old policy, getting
denied. Codex then drafted a broader rule and re-submitted — sound
agent behavior, but not what /wait should provoke. Now /wait captures
the local policy version at start, and after observed-approved waits
for the supervisor to load a strictly-newer version before returning.
Bounded by the caller's deadline; best-effort return if the deadline
elapses without the version bumping. Two new unit tests pin the
happy path and the deadline-clamped fallback.
2. demo.sh auto-approve loop. Replaces approve_when_pending +
wait_for_agent with one approve_pending_until_agent_exits function
that keeps watching for pending chunks and approving them until the
agent process exits (or the configured timeout). Defense in depth
against future redraft scenarios for any reason; today (post-fix#1)
the agent should only submit one proposal per task, but we don't
want to hang silently if it does submit more.
3. UX. Step headers now carry "[t+1.2s]" relative timestamps so reading
the run output makes latency visible (the demo's whole point is the
wait is cheap — surface that). A spin_wait helper renders an ASCII
spinner during the watch loop so the demo never looks frozen on a
TTY. Falls back to plain sleep on non-TTY contexts.
Closes the race condition diagnosed from the trace timing where the
gateway approved at t+0, sandbox observed at t+0.3s, but the supervisor
didn't load v2 until t+9.4s — well after the agent had already retried
and been denied.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
zredlined added a commit that referenced this pull request May 13, 2026
…drafts
Three things in one commit, all surfaced by running the demo end-to-end
against a real gateway and finding the agent had to draft a broader
second proposal.
1. /wait race fix. Previously /wait returned `approved` the moment it
observed the gateway's chunk status flip, but the local supervisor
reloads policy on its own poll cycle (~10s in practice). The agent's
retry would race the reload and hit the still-old policy, getting
denied. Codex then drafted a broader rule and re-submitted — sound
agent behavior, but not what /wait should provoke. Now /wait captures
the local policy version at start, and after observed-approved waits
for the supervisor to load a strictly-newer version before returning.
Bounded by the caller's deadline; best-effort return if the deadline
elapses without the version bumping. Two new unit tests pin the
happy path and the deadline-clamped fallback.
2. demo.sh auto-approve loop. Replaces approve_when_pending +
wait_for_agent with one approve_pending_until_agent_exits function
that keeps watching for pending chunks and approving them until the
agent process exits (or the configured timeout). Defense in depth
against future redraft scenarios for any reason; today (post-fix#1)
the agent should only submit one proposal per task, but we don't
want to hang silently if it does submit more.
3. UX. Step headers now carry "[t+1.2s]" relative timestamps so reading
the run output makes latency visible (the demo's whole point is the
wait is cheap — surface that). A spin_wait helper renders an ASCII
spinner during the watch loop so the demo never looks frozen on a
TTY. Falls back to plain sleep on non-TTY contexts.
Closes the race condition diagnosed from the trace timing where the
gateway approved at t+0, sandbox observed at t+0.3s, but the supervisor
didn't load v2 until t+9.4s — well after the agent had already retried
and been denied.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
zredlined added a commit that referenced this pull request May 13, 2026
* feat(policy): plumb chunk_ids and rejection_reason through proposal pipeline
Prereq plumbing for the agent revise-and-resubmit loop. Two narrow
additive proto changes unblock the upcoming /wait endpoint (#1092),
prover validation badge (#1097), and reject --guidance surfaces (#1098).
- SubmitPolicyAnalysisResponse: add accepted_chunk_ids so the in-sandbox
agent gets handles to watch its proposals. Surfaced through the typed
grpc_client wrapper and policy.local's POST /v1/proposals 202 body.
Closes#1094.
- PolicyChunk + StoredDraftChunk + DraftChunkPayload: add
validation_result (gateway prover verdict, populated by #1097) and
rejection_reason (operator free-form text). Both plain strings; no
enums, no parsing on the read path. Closes#1096.
- RejectDraftChunk now persists the existing reason field into the
chunk's rejection_reason so it round-trips back to the agent via
GetDraftPolicy. UndoDraftChunk clears it on the way back to pending
so consumers cannot read a stale guidance string from a prior reject
-> re-approve -> undo cycle.
Whole surface stays gated behind agent_policy_proposals_enabled. Two
focused tests cover the round-trip and the undo-clears guarantee.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* feat(sandbox): add /v1/proposals/{id} and /wait long-poll to policy.local
The agent feedback channel back from policy.local. Two new routes let
the in-sandbox agent learn its proposal's outcome on a single blocking
HTTP call — zero LLM tokens during the wait.
- GET /v1/proposals/{chunk_id} returns the chunk's current state in one
gateway call.
- GET /v1/proposals/{chunk_id}/wait?timeout=<s> blocks until the chunk
transitions out of pending. Default 60s, clamped [1, 300]. Agent
re-issues on timeout to extend.
Response carries the chunk's status plus the two feedback fields shipped
in the prereq commit: rejection_reason (free-form reviewer text) and
validation_result (gateway prover verdict, empty until #1097). On
timeout: same shape with timed_out: true so the agent can disambiguate
without parsing.
Wait handler short-polls GetDraftPolicy every 1s inside the request with
a tokio::time::Instant deadline. One gateway connection is opened per
request and reused across all polls, so a 60s wait does one TLS
handshake instead of sixty. A future commit can swap the loop body for
a tokio::sync::broadcast driven by a watcher task — the agent-visible
contract (URL, query, response shape) is independent of the polling
implementation.
All routes stay behind agent_policy_proposals_enabled. Closes#1092.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* docs(sandbox): teach policy_advisor skill the wait + redraft loop
The agent-facing instructions for the feedback loop. The endpoints
exist; this is the doc that makes them usable.
policy_advisor.md gains:
- API entries for GET /v1/proposals/{chunk_id} and /wait?timeout=<s>,
including the field semantics (status, rejection_reason,
validation_result, timed_out).
- A note on the submit response's accepted_chunk_ids /
rejection_reasons split so the agent handles partial acceptance.
- Step 6 saves the chunk_ids and addresses any submit-time rejections
before waiting.
- Step 7 walks the four wait outcomes: approved (retry, with the
honest "may still fail" caveat), rejected (read rejection_reason
AND validation_result; address whichever has content), still-pending
with timed_out (re-call), non-2xx (surface, do not retry).
skills.rs gains two assertions on the skill content so a future edit
cannot drop the wait endpoint or the rejection_reason directive
silently.
Closes#1095.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* test(policy-advisor): add end-to-end smoke for the agent feedback loop
A focused smoke that exercises the new policy.local /wait endpoint
on a live gateway + sandbox, separate from the existing no-LLM
regression harness (which still drives the OLD retry-with-bash-loop
recovery pattern).
Two flows:
- Flow A — approve-and-retry: agent submits, /wait blocks, host runs
`openshell rule approve`, /wait returns status=approved. Confirms
the happy path round-trip latency.
- Flow B — reject-with-guidance: agent submits, /wait blocks, host
runs `openshell rule reject --reason "..."`, /wait returns
status=rejected with the exact reviewer text in rejection_reason.
Confirms the free-form guidance contract round-trips through the
agent feedback channel.
No GitHub credentials needed — proposals are synthetic and never
trigger outbound traffic. Both flows expect agent_policy_proposals_enabled=true
and a running gateway.
Adds three cases to sandbox-runner.sh: submit-test-proposal (no GH
deps), proposal-status, proposal-wait. The existing put-file and
submit-proposal cases used by test.sh are untouched.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(policy-advisor): surface real CLI errors from wait-smoke preflight
The preflight piped openshell's stderr to /dev/null and relied on jq to
default the missing setting key to "<unset>", but under `set -euo
pipefail` a non-zero exit from openshell makes the whole pipeline fail
and the command substitution exits the script silently before the
intended fail() message can print.
Capture stderr explicitly, check the CLI exit code, and surface the
real error plus the expected fix (port-forward + gateway add + select)
when the CLI cannot reach the gateway.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(policy-advisor): pass --json to settings get in wait-smoke preflight
`openshell settings get --global` defaults to a human-readable table;
jq cannot parse it and the preflight died with a numeric-literal error.
Pass --json so jq gets actual JSON. Also touched up the suggested
recovery commands in the preflight error to match the real CLI shape
(`gateway add <endpoint> --name <name>` and the env-var override
warning).
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(policy): dedup draft chunks only in mechanistic mode; return effective id
The smoke harness for the agent feedback loop caught a real bug in the
gateway: SubmitPolicyAnalysis's response carried a chunk_id that was
never persisted whenever the SQL ON CONFLICT path fired. Two failure
modes, both load-bearing:
- Agent-authored proposals targeting the same host/port/binary
(e.g. the redraft-after-rejection loop) silently folded into one row
and any RejectDraftChunk by the new chunk_id failed with "chunk not
found." Latent since #1151, surfaced by #1094 returning chunk_ids.
- Mechanistic mode had the same class of bug — the dedup fold-in is
the intended behavior there, but the response still advertised the
newly-generated UUID instead of the existing row's id. Less visible
because no current caller reads mechanistic chunk_ids back, but the
proto contract was violated either way.
Fix in three parts:
- put_draft_chunk now takes Option<&str> dedup_key explicitly and
returns the effective row id (via RETURNING). None binds NULL to the
dedup_key column, which bypasses the partial-index ON CONFLICT path
entirely. Caller-decides semantics replace store-side magic.
- handle_submit_policy_analysis picks dedup_key per chunk using an
allowlist (only "mechanistic" dedups) and pushes the returned
effective_id to accepted_chunk_ids. New modes default to no-dedup so
a misconfigured caller cannot silently lose proposals.
- The two-copy draft_chunk_dedup_key helper consolidated to one
observation_dedup_key in policy_store.rs with a doc comment.
Tests:
- agent_authored_submits_for_same_endpoint_do_not_dedup pins the
redraft-loop contract: two intentional submissions with the same
host/port/binary get distinct chunk_ids, both findable via
GetDraftPolicy, both rejectable by id.
- mechanistic_submits_for_same_endpoint_dedup_into_one_chunk locks in
the observation-mode dedup AND asserts both submits return the same
effective_id — would have caught the deeper bug.
Proto: SubmitPolicyAnalysisRequest.analysis_mode doc updated to
describe the actual semantics (mechanistic dedups, agent_authored and
unknown modes do not).
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* docs(examples): retarget policy-management demo at the /wait endpoint
The narrated demo (examples/agent-driven-policy-management) has been the
public face of this feature since #1151. Its agent prompt told Codex to
retry the original PUT every few seconds for up to 120 seconds — a
polling workaround for the missing /wait endpoint that this branch
shipped. Update the demo to exercise /wait so the canonical reading of
the feature reflects the actual UX win.
- agent-task.md: step 4 is now "call /wait, branch on status" with the
three outcomes spelled out (approved → retry once; rejected → read
rejection_reason and revise or stop; pending+timed_out → re-issue
/wait once, do NOT busy-loop or shorten the timeout). Also makes
explicit that the demo submits one rule per proposal so
accepted_chunk_ids[0] is the safe single id to wait on.
- demo.sh: header docstring rewritten as a six-step loop that mirrors
the README. narrate_sandbox_workflow drops its parallel numbering and
uses bullets (the runtime narration is the agent's sub-actions, not a
separate decomposition of the loop). Approve step header and success
message now reference /wait waking the agent, not "policy hot-reload
retry."
- README.md: top-of-file flow expanded from 5 to 6 steps to include the
/wait call and chunk_id capture; "Going further" section now describes
both regression scripts and the boundary between them (real-GitHub
retry vs. synthetic /wait wire test). Slow-path qualifier corrected
from "image pull on first run" to "sandbox cold-start (SSH bring-up
plus Codex install)".
- wait-smoke.sh header rewritten to make it unambiguous this is a
regression, NOT a tutorial, with explicit prereq commands instead of
prereq descriptions, and a pointer at demo.sh for the narrated story.
No code paths change; this is the readability pass.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(examples): pass --yes on demo.sh's global setting writes
Global setting updates require explicit confirmation in non-interactive
mode; demo.sh's enable_agent_proposals and the cleanup restore path
were missing --yes and hard-failed the preflight. Pre-existing issue
that surfaced now that more of the demo runs through this path.
No other behavior change.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* feat(sandbox): emit OCSF audit events for policy proposal lifecycle
The demo's policy decision trace previously showed only the proxy
enforcement story (HTTP:PUT DENIED, CONFIG:LOADED, HTTP:PUT ALLOWED).
It was silent about who proposed what or who decided what — the
audit-trail receipts for the agent feedback loop were missing.
policy.local now emits sandbox-side OCSF events at the observation
moments, into the same stream as the existing CONFIG:LOADED:
- CONFIG:PROPOSED on submit_proposal acceptance. Per accepted chunk:
the message names the chunk_id, target endpoint, L7 method/path, and
binary so the trace correlates against the inbox card via chunk_id.
- CONFIG:APPROVED on /wait observation of approved status.
- CONFIG:REJECTED on /wait observation of rejected status. Carries the
reviewer's free-form rejection_reason in the message AND as an
unmapped field, both sanitized (control chars stripped, capped at
200 chars with an ellipsis marker). The agent still reads the raw
text via GET /v1/proposals/{id}; sanitization is audit-side only,
per AGENTS.md's no-secrets-in-OCSF rule.
The submit path defends the audit_summaries / accepted_chunk_ids
index pairing against a future gateway change that compresses past
rejected chunks (the proto doesn't promise 1:1 ordering with the
request). Today client-side validation makes the lengths always
match; if they don't, the pairing falls back to a generic per-id
event rather than mis-attribute.
The wait handler's emit site fires once per terminal-status
observation. Multiple concurrent waiters on the same chunk would
each emit one event; acceptable for single-waiter-per-chunk demos
and the right place to dedup is the SIEM.
demo.sh's trace filter now surfaces the four CONFIG: events
alongside HTTP:PUT, so the trace at the end of every run tells the
full story from deny to allow via propose -> approve.
wait-smoke.sh's prereq notes recommend redirecting kubectl
port-forward output so its "Handling connection for 8090" lines
don't bleed into demo narration.
Three new unit tests on the sandbox-side helpers — summary builder
happy path, fallback, and the rejection_reason sanitizer.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* feat(policy): /wait awaits local policy reload; demo auto-approves redrafts
Three things in one commit, all surfaced by running the demo end-to-end
against a real gateway and finding the agent had to draft a broader
second proposal.
1. /wait race fix. Previously /wait returned `approved` the moment it
observed the gateway's chunk status flip, but the local supervisor
reloads policy on its own poll cycle (~10s in practice). The agent's
retry would race the reload and hit the still-old policy, getting
denied. Codex then drafted a broader rule and re-submitted — sound
agent behavior, but not what /wait should provoke. Now /wait captures
the local policy version at start, and after observed-approved waits
for the supervisor to load a strictly-newer version before returning.
Bounded by the caller's deadline; best-effort return if the deadline
elapses without the version bumping. Two new unit tests pin the
happy path and the deadline-clamped fallback.
2. demo.sh auto-approve loop. Replaces approve_when_pending +
wait_for_agent with one approve_pending_until_agent_exits function
that keeps watching for pending chunks and approving them until the
agent process exits (or the configured timeout). Defense in depth
against future redraft scenarios for any reason; today (post-fix#1)
the agent should only submit one proposal per task, but we don't
want to hang silently if it does submit more.
3. UX. Step headers now carry "[t+1.2s]" relative timestamps so reading
the run output makes latency visible (the demo's whole point is the
wait is cheap — surface that). A spin_wait helper renders an ASCII
spinner during the watch loop so the demo never looks frozen on a
TTY. Falls back to plain sleep on non-TTY contexts.
Closes the race condition diagnosed from the trace timing where the
gateway approved at t+0, sandbox observed at t+0.3s, but the supervisor
didn't load v2 until t+9.4s — well after the agent had already retried
and been denied.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(sandbox): /wait detects policy reload by content, not the schema version
The previous attempt at the /wait-after-approve race fix compared
`SandboxPolicy.version` between /wait start and the policy reload —
but that field is the *schema* version (constant 1), not a revision
counter. Every comparison was `current(1) > baseline(1) == false`, so
the wait blocked until the agent's 300s timeout regardless of whether
the supervisor had actually reloaded. The demo SSH connection then
timed out around the 240s mark.
Diagnosed from a live run's OCSF trace: supervisor pulled v2 at
+8.5s after approval (CONFIG:LOADED), but the sandbox-side
CONFIG:APPROVED that my /wait emits didn't fire until +304s — exactly
at the 300s deadline.
Fix: compare the whole policy via prost's derived PartialEq. Any
field change (network_policies map being the only one that actually
mutates today) flips equality. A clone-per-200ms-tick on a few-KB
proto is cheap inside the bounded wait window.
Tests rewritten to match the new contract: the supervisor-reload
fixture now keeps `version: 1` constant and changes `network_policies`
contents, mirroring the exact failure mode from the live run.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(examples): redact tokens with python literal-string replace, not sed
The sed-based redact_log in demo.sh broke when one of the auth tokens
contained a character that conflicted with sed's pattern parser
("unterminated substitute pattern" on the Codex JWT). The whole log
tail then blanks on failure, hiding the very failure context we're
trying to surface.
Switch to a python subprocess that takes the tokens via argv and does
literal str.replace. No regex, no delimiter games, no truncation.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(sandbox): scope /wait reload check to the approved rule
Reviewer (John Myers) flagged two failure modes in the prior whole-policy
fingerprint approach used by policy.local /wait:
- False sleep: when the supervisor reloads between two /wait calls
(the skill tells the agent to re-issue on timed_out), the new call
snapshots the already-updated policy as baseline and burns the full
timeout waiting for a change that never comes.
- False wakeup: any unrelated reload (other agent's approval, settings
change) flips the diff, but the chunk's actual rule may not be loaded
yet — the agent retries and hits policy_denied for no real signal.
Replace the diff with rule-coverage. New public helper
openshell_policy::policy_covers_rule reuses endpoints_overlap (so it
matches add_rule's merge semantics, including the fold-into-existing-key
case) plus an L7 allow check on method/path (so an existing endpoint
that doesn't yet contain the proposed method doesn't signal coverage).
Add policy_reloaded: true|false to the /wait response on approve, with
a 500ms floor on the reload-wait phase so approvals arriving near the
deadline still get a fair shot at reloaded=true. Update the
policy_advisor skill to branch on it: reloaded=true → retry;
reloaded=false → re-issue /wait once with timeout=30, then surface to
user. Don't loop tightly.
Tests:
- 9 new unit tests in openshell-policy pinning coverage semantics
(L4-only, L7 method gap, fold-into-existing-key, empty binaries).
- 4 new tokio tests in policy_local mirroring John's exact scenarios.
- wait-smoke.sh asserts policy_reloaded=true on Flow A.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(server): make GetDraftPolicy dual-auth so /wait works under OIDC
policy.local calls GetDraftPolicy from inside the sandbox supervisor
via the sandbox gRPC client, which authenticates with the shared
x-sandbox-secret. GetDraftPolicy was listed only in the Bearer-auth
scope table (config:read) and was not in SANDBOX_SECRET_METHODS or
DUAL_AUTH_METHODS, so OIDC-enabled gateways rejected those calls and
the /wait long-poll surfaced gateway_lookup_failed. Local/no-OIDC
setups happened to work because the auth check is short-circuited.
Add GetDraftPolicy to DUAL_AUTH_METHODS, matching the existing
GetSandboxConfig pattern (called by both CLI reviewer surfaces with
Bearer and the sandbox supervisor with x-sandbox-secret). Dual-auth
short-circuits the scope check for sandbox-secret callers, so the
config:read entry in authz.rs continues to gate Bearer-only flows.
Mirror the openshell_get_sandbox_config_is_dual_auth assertion for
GetDraftPolicy.
Note: ssh_handshake_secret is server-wide, not per-sandbox, so a
sandbox-secret caller can today name any sandbox in a SubmitPolicyAnalysis
request — and now in a GetDraftPolicy request. The exposure is
symmetric with the existing SANDBOX_SECRET_METHODS pattern. Filed as a
follow-up: per-sandbox secret binding, tracked separately.
Signed-off-by: Alexander Watson <zredlined@gmail.com>
* fix(ci): address rebased check failures
Signed-off-by: Alexander Watson <zredlined@gmail.com>
---------
Signed-off-by: Alexander Watson <zredlined@gmail.com>
copy-pr-botBot pushed a commit that referenced this pull request Aug 5, 2026
- Remove dead boolCount function that would fail golangci-lint (#1)
- Emit EventAdded for the first watch event instead of EventModified,
matching k8s watch semantics (#7)
- Add mutex locking to all mock server methods that access the shared
sandboxes map, fixing latent race conditions (#12)
- Skip HealthCheck integration test that calls an unimplemented stub (#13)
- Scope doc.go examples: mark sections for sub-clients not yet available
in this PR with "available in a future release" (#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
as reserved for future use (#2, #6)
Signed-off-by: Roland Huß <rhuss@redhat.com>
copy-pr-botBot pushed a commit that referenced this pull request Aug 5, 2026
- Remove dead boolCount function that would fail golangci-lint (#1)
- Emit EventAdded for the first watch event instead of EventModified,
matching k8s watch semantics (#7)
- Add mutex locking to all mock server methods that access the shared
sandboxes map, fixing latent race conditions (#12)
- Skip HealthCheck integration test that calls an unimplemented stub (#13)
- Scope doc.go examples: mark sections for sub-clients not yet available
in this PR with "available in a future release" (#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
as reserved for future use (#2, #6)
Signed-off-by: Roland Huß <rhuss@redhat.com>
varshaprasad96 pushed a commit to varshaprasad96/OpenShell that referenced this pull request Aug 5, 2026
…VIDIA#2271)
* feat(sdk/go): add Go SDK foundation, types, and sandbox client (A)
Add the Go SDK module with the full API contract and a working sandbox
client as the first vertical slice. All other resource clients are present
as stubs returning Unimplemented errors, to be replaced with real
implementations in subsequent PRs.
Contents:
- Module setup (go.mod, Makefile, mise.toml)
- All domain types (types/ package)
- Full ClientInterface with all sub-client accessors
- Shared infrastructure (errors, auth, gRPC connection, logging)
- Sandbox client with converter and tests (fully functional)
- Stub clients for remaining resources (exec, file, health, provider,
profile, config, refresh, policy, service, ssh, tcp)
Part of the Go SDK decomposition plan (NVIDIA#2270).
Implements NVIDIA#2044.
* fix(sdk/go): address review feedback on PR NVIDIA#2271
- Make scheme parsing drive transport selection: http:// uses plaintext
gRPC, https:// or no scheme uses TLS. Add regression tests.
- Add Resources and DriverConfig fields to SandboxTemplate and update
both converter directions (SandboxFromProto/SandboxSpecToProto).
- Regenerate proto bindings from current canonical proto sources to
eliminate drift (SigV4/MCP fields, params matchers, reserved fields).
- Run gofmt/goimports on all handwritten Go files.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): address principal engineer review findings
- Remove dead boolCount function that would fail golangci-lint (NVIDIA#1)
- Emit EventAdded for the first watch event instead of EventModified,
matching k8s watch semantics (NVIDIA#7)
- Add mutex locking to all mock server methods that access the shared
sandboxes map, fixing latent race conditions (NVIDIA#12)
- Skip HealthCheck integration test that calls an unimplemented stub (NVIDIA#13)
- Scope doc.go examples: mark sections for sub-clients not yet available
in this PR with "available in a future release" (NVIDIA#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
as reserved for future use (NVIDIA#2, NVIDIA#6)
Signed-off-by: Roland Huß <rhuss@redhat.com>
* refactor(sdk/go): migrate mise config to centralized task include
Move Go SDK mise configuration from standalone sdk/go/mise.toml into
the project's centralized pattern:
- Add Go tools (go, golangci-lint, protoc-gen-go, protoc-gen-go-grpc)
to root mise.toml [tools] section
- Create tasks/go.toml with all SDK tasks using go: namespace prefix
and dir=sdk/go for working directory
- Update sdk/go/Makefile to reference namespaced task names
- Update proto:sync default path for monorepo layout
Addresses review feedback from drew on PR NVIDIA#2271 regarding mise
convention alignment.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* refactor(sdk/go): remove UPSTREAM_VERSION standalone repo artifact
Remove sdk/go/proto/UPSTREAM_VERSION file and its exclusion from
proto:check. This was a leftover from the standalone repo prototype.
In a monorepo, proto drift is detectable via git diff between
sdk/go/proto/ and proto/ directly.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* refactor(sdk/go): switch proto generation from protoc to buf
Replace raw protoc invocations with buf for Go SDK proto code generation,
aligning with the TS SDK approach (PR NVIDIA#2122).
- Add repo-level buf.yaml declaring proto/ as the buf module with lint
and breaking change detection config
- Add sdk/go/buf.gen.yaml configuring buf to generate Go code directly
from root proto/ (no more vendored .proto copies)
- Delete vendored .proto source files from sdk/go/proto/
- Rewrite go:proto:gen and go:proto:check mise tasks to use buf
- Remove go:proto:sync and go:proto:clean tasks (no longer needed)
- Add proto target to sdk/go/Makefile
- Add buf 1.72.0 to root mise.toml tool dependencies
- Include options.proto in generation (was stripped from vendored copies)
- Regenerate all .pb.go files via the new buf pipeline
Signed-off-by: Roland Huß <rhuss@redhat.com>
* test(sdk/go): add proto-converter field coverage detection
Use protobuf reflection to enumerate all fields on key proto messages
(SandboxSpec, SandboxTemplate, SandboxStatus, SandboxCondition,
SandboxPolicy) and compare against explicit handled/skipped sets in the
converter tests.
Unhandled fields produce warnings (t.Log), not failures, so proto
contributors are not forced to fix SDK converters in the same PR. Stale
entries in the handled set (removed proto fields) do fail, since they
indicate the converter references something that no longer exists.
A follow-up CI workflow will create GitHub issues when converter drift
lands on main.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): bump Go to 1.26 and fix errcheck lint violations
The upstream go.mod now has `toolchain go1.26.4`, which requires Go 1.26
to build golangci-lint. Bump the mise.toml Go version from 1.25 to 1.26
and wrap deferred Close() calls in test helpers to satisfy errcheck.
Assisted-By: 🤖 Claude Code
* feat(sdk/go): add ObjectMeta fields (annotations, workspace, deletion_timestamp)
Add three new proto ObjectMeta fields to Sandbox and Provider domain
types: Annotations (map), Workspace (string), and DeletionTimestamp
(*time.Time). Update converters in both directions, deep-copy maps at
the proto/SDK boundary, and add TimeFromMillisPtr/MillisFromTimePtr
helper functions.
Assisted-By: 🤖 Claude Code
* chore(sdk/go): regenerate proto bindings after rebase
Pick up workspace fields from upstream PR NVIDIA#2445 (Wire authorization
into workspace model). All request messages now include workspace
parameter in the generated Go bindings.
Assisted-By: 🤖 Claude Code
* feat(sdk/go): add workspace scoping to all RPC interfaces
Add workspace parameter to every sandbox-scoped RPC method across all
interfaces (Sandbox, Exec, File, Service, SSH, TCP, Config, Policy,
Provider, Profile, Refresh). The workspace string is passed as the
second parameter after ctx, following the convention workspace then
resource-name.
Key changes:
- SandboxInterface: all 10 methods gain workspace parameter
- sandbox_client.go: passes Workspace field in every proto request
- ListOptions: add AllWorkspaces field for cross-workspace queries
- All stub interfaces updated to match new signatures
- All sandbox client tests updated with "default" workspace
Assisted-By: 🤖 Claude Code
* chore(sdk/go): remove coverage.out from tracking
Assisted-By: 🤖 Claude Code
* fix(sdk/go): address review feedback from mrunalp
- Add RefreshStrategyAWSStsAssumeRole to match proto enum value 6,
fulfilling the "all domain types upfront" contract
- Wrap context.DeadlineExceeded and context.Canceled in StatusError
so IsDeadlineExceeded() and IsCancelled() helpers work correctly
- Return error from mapToStruct/SandboxSpecToProto instead of silently
discarding structpb.NewStruct failures on invalid template maps
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): address remaining review items
- Wire go:ci into root ci task so SDK is tested in repository CI
- Fix gofmt formatting on converter files
- Add goimports to mise.toml tools
- Add coverage.out to .gitignore
- Add Go SDK section to AGENTS.md and CONTRIBUTING.md
- Add regression tests for context-error wrapping (IsDeadlineExceeded,
IsCancelled) and invalid template map rejection
- Remove panic from SandboxToProto, return error instead
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): pin goimports version and update lockfile
Pin goimports to 0.48.0 instead of "latest" and regenerate mise.lock
to include the new entry.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): TLS.Insecure means skip-verify, not plaintext
Align TLS.Insecure semantics with the Rust SDK: Insecure: true now
uses TLS with InsecureSkipVerify (skip cert verification) instead of
switching to plaintext. Only the http:// scheme triggers plaintext.
This fixes token auth against dev/k3d gateways: StaticToken and
RefreshableToken require transport security, which real TLS (even
with InsecureSkipVerify) satisfies, but plaintext does not.
For http:// + token auth (dev gateways without TLS), wrap the auth
provider to override RequireTransportSecurity, matching the Rust
SDK's behavior where http:// accepts any auth mode.
Transport decision table (matches Rust SDK crates/openshell-sdk):
http:// + any TLS config -> plaintext (TLS config ignored)
https:// + Insecure: true -> TLS, skip cert verify
https:// + Insecure: false -> TLS, full verification
no scheme -> same as https://
Signed-off-by: Roland Huss <rhuss@redhat.com>
* feat(sdk/go): add missing policy proto fields
Add 6 previously silently dropped fields to the network policy types
and converters, preventing security-relevant data loss on round-trip:
NetworkEndpoint fields 19-23:
- CredentialSigning: SigV4 re-signing mode
- SigningService: AWS service name for SigV4
- SigningRegion: AWS region override for SigV4
- JsonRpcMaxBodyBytes: JSON-RPC body inspection limit
- Mcp: MCP-specific policy options (new McpOptions type)
L7Allow and L7DenyRule field 9:
- Params: MCP params matcher map for tools/call filtering
New type McpOptions with StrictToolNames and AllowAllKnownMcpMethods
optional booleans matching the proto definitions.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): enforce coverage test and extend to policy messages
Change coverage_test.go from t.Logf (silent) to t.Errorf so that
unhandled proto fields fail the test immediately. Add coverage tests
for NetworkEndpoint (23 fields), L7Allow (8 fields), L7DenyRule
(8 fields), and McpOptions (2 fields).
Any new proto field that is not in the handled set or explicitly
skipped now breaks the build, closing the silent-drift gap.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* ci(sdk/go): add Go SDK job to branch-checks workflow
Add a Go SDK job to branch-checks.yml that runs mise run go:ci
(lint, build, test, proto-check, docs-check) on every PR. This
ensures the SDK is tested in CI, not just locally.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* fix(sdk/go): address should-fix review items
NVIDIA#6 Fix broken godoc examples: add workspace parameter to all method
calls in doc.go that were broken after workspace scoping.
NVIDIA#7 Add Err field to Event[T]: Watch error events now carry the
underlying error instead of discarding it.
NVIDIA#8 Separate Unauthenticated from PermissionDenied: add
ErrorUnauthenticated code and IsUnauthenticated() helper. gRPC
Unauthenticated (401) now maps to its own code instead of
collapsing into PermissionDenied (403).
NVIDIA#9 Add Unwrap to StatusError: replace dead Details field with Cause
error field. StatusError.Unwrap() returns Cause, enabling
errors.Is/As unwrapping. FromGRPCError and contextError both
populate Cause.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* ci(sdk/go): add go:format:check to CI pipeline
Add gofmt format verification to go:ci. Catches unformatted Go files
before they reach the PR. Fix formatting on coverage_test.go.
Signed-off-by: Roland Huss <rhuss@redhat.com>
* chore(sdk/go): remove Makefile in favor of mise tasks
All build, lint, test, and proto-gen tasks are already defined in
tasks/go.toml and invoked via mise. The Makefile was a leftover
that duplicated this and raised questions in review.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* feat(sdk/go): sync proto bindings and add credential handle support
Regenerate Go proto bindings after rebase to pick up new
CredentialHandle message and Provider.credential_handles and
profile_workspace fields from upstream. Add domain types, converter
support, and proto field coverage tests for Provider and
CredentialHandle.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): reject plaintext auth leak and fix watch error handling
Reject http:// addresses when the auth provider requires transport
security instead of silently stripping the requirement. Remove the
insecureAuthWrapper that overrode RequireTransportSecurity.
Fix watch stream error handling: use blocking send for terminal
errors so they are never silently dropped when the channel is full,
and wrap mid-stream errors with converter.FromGRPCError so SDK error
helpers like IsUnavailable work on watch Event.Err.
Signed-off-by: Roland Huß <rhuss@redhat.com>
* fix(sdk/go): address review findings from multi-agent code review
- WaitReady now detects SandboxDeleting phase and returns immediately
instead of polling indefinitely
- Watch goroutine defers streamCancel() to prevent context leaks
- Fix StopOnTerminal=false test to keep stream open (was wrong-reason
pass due to stream ending, not StopOnTerminal logic)
- Add EventDeleted test covering the Deleting phase branch
- Add provider converter unit tests for CredentialHandle round-trip,
nil handling, and empty maps
Signed-off-by: Roland Huß <rhuss@redhat.com>
---------
Signed-off-by: Roland Huß <rhuss@redhat.com>
Signed-off-by: Roland Huss <rhuss@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@drew@johntmyers