Skip to content

fix(proxy): normalize trailing-dot CONNECT hosts before policy evaluation - #2

Closed
letv1nnn wants to merge 7 commits into
mainfrom
2201-normalize-trailing-dot-host
Closed

fix(proxy): normalize trailing-dot CONNECT hosts before policy evaluation#2
letv1nnn wants to merge 7 commits into
mainfrom
2201-normalize-trailing-dot-host

Conversation

@letv1nnn

@letv1nnn letv1nnn commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

Fixes inconsistent handling of trailing-dot hostnames (e.g. api.example.com.) in the sandbox supervisor's CONNECT proxy. Such hosts are valid RFC 1034 FQDNs used in Kubernetes ndots:5 environments to bypass search-domain expansion, but were treated as distinct from their undotted equivalents across policy evaluation, middleware selection, and credential matching.

Changes

  • Added normalize_connect_host() helper that strips exactly one trailing dot from the CONNECT authority host
  • Applied normalization at the call site in handle_tcp_connection — policy evaluation, middleware selection, and logging all receive the canonical (undotted) form
  • resolve_and_check_trusted_gateway and resolve_and_check_allowed_ips receive the original raw_host to preserve root-anchored DNS semantics
  • Added unit tests covering: single trailing dot stripped, clean host unchanged, double-dot strips only one (remains invalid downstream)

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

johntmyers and others added 7 commits July 13, 2026 08:52
@johntmyers
fix(tui): redraw after sandbox shell exits (NVIDIA#2230)
88f2656 
* fix(tui): redraw after sandbox shell exits

Closes NVIDIA#2229

Render the restored terminal before synchronous gateway refreshes and propagate terminal lifecycle failures from both SSH handoff paths.

Signed-off-by: John T. Myers <9696606+johntmyers@users.noreply.github.com>

* fix(tui): restore input after sandbox shell

Discard stale events accumulated around the suspended TUI and let the normal periodic tick refresh state after resuming.

Signed-off-by: John T. Myers <9696606+johntmyers@users.noreply.github.com>

---------

Signed-off-by: John T. Myers <9696606+johntmyers@users.noreply.github.com>
@rhuss
fix(agents): add confirmation gate to triage-issue batch mode (NVIDIA…
0fe24a4 
…#2239)

Batch triage previously processed all state:triage-needed issues
immediately without any confirmation. This led to accidental
mass-commenting (29 triage comments on a public repo) when the
skill was invoked by mistake.

Add a mandatory preview-and-confirm step: the agent must show the
issue count and titles, then ask for explicit user confirmation
before posting any comments.

Single-issue mode is unchanged.

Assisted-By: 🤖 Claude Code

Signed-off-by: Roland Huß <rhuss@redhat.com>
@johntmyers
fix(gator): retry review after draft blocker clears (NVIDIA#2200)
9ad53b3 
Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
@gracesmith6504
fix(certgen): stage temp dir inside output dir to fix cross-device re…
4e1ffef 
…name (NVIDIA#2241)

generate-certs stages temp files beside --output-dir using
dir.with_file_name(). When --output-dir is a container or WSL volume
mount, the staging dir lands on a different filesystem and
std::fs::rename fails with EXDEV.

Move staging inside the output dir so the rename always stays on the
same filesystem, preserving atomic replacement.

Fixes NVIDIA#2173

Signed-off-by: Grace Smith <grasmith@redhat.com>
@shiju-nv
refactor(jsonrpc): carry typed inspection errors (NVIDIA#2244)
fcc9db3 
Signed-off-by: Shiju <shiju@nvidia.com>
@johntmyers
docs(agents): add gator launch skill (NVIDIA#2203)
ee9b455 
* docs(agents): add gator launch skill

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(agents): narrow gator launch trigger

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(agents): trim gator launch skill intro

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(agents): harden gator launch examples

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(agents): avoid assuming gator gateway

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(agents): trim gator launch rules

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

---------

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
@letv1nnn
fix(proxy): normilize trailing-dot host
9b95e11
@letv1nnn
letv1nnn force-pushed the 2201-normalize-trailing-dot-host branch from 95a5648 to 9b95e11 Compare July 13, 2026 17:43
@letv1nnn letv1nnn closed this Jul 13, 2026
@letv1nnn
letv1nnn deleted the 2201-normalize-trailing-dot-host branch July 13, 2026 18:34
letv1nnn pushed a commit that referenced this pull request Aug 7, 2026
@rhuss
feat(sdk/go): add Go SDK foundation, types, and sandbox client (A) (N…
c5f8366 
…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 (#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 (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" (#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
  as reserved for future use (#2, #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

#6 Fix broken godoc examples: add workspace parameter to all method
   calls in doc.go that were broken after workspace scoping.

#7 Add Err field to Event[T]: Watch error events now carry the
   underlying error instead of discarding it.

#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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@letv1nnn @johntmyers @rhuss @gracesmith6504 @shiju-nv