Skip to content

feat: mediate vault login -method=oidc (custom inject header + per-command open_port) - #1476

Merged
SequeI merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/vault-oidc-inject-header
Jul 27, 2026
Merged

feat: mediate vault login -method=oidc (custom inject header + per-command open_port)#1476
SequeI merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/vault-oidc-inject-header

Conversation

@kipz

@kipz kipz commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1473

Summary

Enables mediating HashiCorp Vault's vault login -method=oidc so the real Vault token never enters the sandbox — captured at the OIDC callback, replaced with a phantom, redeemed on egress to the Vault API. Three generic oauth_capture/policy gaps blocked the Vault CLI flow:

  • Optional inject_header / credential_format on oauth_capture providers — Vault authenticates with X-Vault-Token: <raw token>, not Authorization: Bearer {}. Defaults unchanged; set X-Vault-Token / {} for Vault. Validated at parse time: credential_format must contain {}; inject_header must be a valid HTTP header token.
  • request_nonce_fields now optional — the Vault OIDC callback (/v1/auth/<mount>/oidc/callback) is capture-only: the token arrives in the response and is never re-sent in a request body. The field only marks where to swap a phantom by value in an outbound refresh body — it does not gate capture, and api_hosts is mandatory, so omitting it is safe.
  • open_port / open_port_range on per-command network policies — so the mediated vault command can bind its localhost:8250 OIDC callback listener. Mirrors the top-level network.open_port, wired through add_policy_network (macOS + Linux) via the existing localhost_port_ranges path (enforceable on macOS, unlike tcp_bind_ports).

Agent Disclosure

This PR was authored by an AI agent (disclosed on #1473).

  • Consulted: credential_provider.rs, proxy_runtime.rs, command_policy.rs, tool-sandbox/platform/{macos,linux}.rs, oauth_capture/{rewrite,endpoint}.rs, and CLAUDE.md.
  • Composes with existing mechanisms (add_localhost_port_range -> proxy_bind_port_ranges; the api_hosts route builder) rather than adding new machinery.
  • No unwrap/expect in new code; uses NonoError; adds no new filesystem paths.

Test Plan

  • make ci clean (fmt, clippy -D warnings, tests) on a fresh rebase onto upstream/main.
  • New unit tests: custom inject_header on the api_hosts route; open_port merge + parse round-trip; open_port -> localhost bind ranges (macOS + Linux); credential_format / inject_header validation.
  • Verified end-to-end against a real vault login -method=oidc: auth.client_token captured at the callback and replaced with a phantom, the sandboxed vault only ever holds the phantom, and it is redeemed to the real token on egress to the Vault API.
  • Docs updated: sandboxed-oauth-logins.mdx (new fields + Vault callback-bind note) and tool-sandbox.mdx (open_port reference).

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates
  • Release note has been added to CHANGELOG.md if needed

Agent Compliance Check

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I disclosed that I am an agent in the issue discussion
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths
  • This PR matches the approved or disclosed issue scope

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +384
Lines removed -8
Total changed 392
Classification Large (> 300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@kipz
kipz marked this pull request as ready for review July 21, 2026 15:57

@nogent-nolabs-ai nogent-nolabs-ai 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.

🛡️ nogent could not produce a schema-conforming security review for this change (the model output failed canary/shape validation). A maintainer should review manually.

kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
Integration fixup: nolabs-ai#1469/nolabs-ai#1443 added CredentialRouteDef.upgrades;

Signed-off-by: James Carnegie <me@kipz.org>
nolabs-ai#1476's vault test literal predates it. Combined-branch only.
Comment on lines +192 to +209
if provider
.credential_format
.as_deref()
.is_some_and(|format| !format.contains("{}"))
{
return Err(NonoError::ProfileParse(format!(
"credential_providers.{name}.credential_format must contain the '{{}}' token placeholder"
)));
}
if provider
.inject_header
.as_deref()
.is_some_and(|header| !is_valid_http_header_name(header))
{
return Err(NonoError::ProfileParse(format!(
"credential_providers.{name}.inject_header must be a valid HTTP header name (RFC 7230 token)"
)));
}

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.

looks good @kipz , just one small extra worth getting in. credential_format's value template isn't CRLF-checked but the header name is, and it flows through the same raw-string path. It's operator-supplied/trusted, so it's a consistency/defense-in-depth nit, not a blocker

perhaps something like this on line 200 of your last commit (above the check for .inject_header):

if provider
    .credential_format
    .as_deref()
    .is_some_and(|format| format.bytes().any(|b| matches!(b, b'\r' | b'\n' | b'\0')))
{
    return Err(NonoError::ProfileParse(format!(
        "credential_providers.{name}.credential_format must not contain control characters (CR, LF, NUL)"
    )));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lukehinds done in 2c84b73. I widened the guard while there: rejects all HTTP field-value control bytes except horizontal tab (b.is_ascii_control() && b != b'\t'), not just CR/LF/NUL. added tests plus a positive case that tab is allowed.

kipz added a commit to kipz/nono that referenced this pull request Jul 23, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 23, 2026
Add format (nolabs-ai#1489) and upgrades (nolabs-ai#1443) fields to vault-oidc (nolabs-ai#1476) test
initializers that only one side of the merge reconciled.

Signed-off-by: James Carnegie <me@kipz.org>
@kipz
kipz force-pushed the kipz/vault-oidc-inject-header branch from 2c84b73 to 9b1690c Compare July 24, 2026 15:41
@lukehinds lukehinds added this to the 0.70 milestone Jul 27, 2026
kipz added 2 commits July 27, 2026 13:55
oauth_capture providers can set inject_header/credential_format for the
api_hosts redemption route (default stays Authorization/Bearer {}), and
request_nonce_fields is optional for capture-only token endpoints. This
lets credentials for non-Bearer APIs (raw token in a custom header) be
brokered.

Command network policies gain open_port/open_port_range, wired through
add_policy_network on macOS and Linux via the localhost-port-range path,
so a proxy-routed child command can bind a localhost OAuth callback
listener the way the top-level network policy already allows.

Signed-off-by: James Carnegie <me@kipz.org>
The format template flows into a raw header value on the redemption
route, so a stray control byte enables header injection. Reject all HTTP
field-value control characters (everything except horizontal tab),
matching the guard already applied to the header name.

Signed-off-by: James Carnegie <me@kipz.org>
@SequeI
SequeI force-pushed the kipz/vault-oidc-inject-header branch from 9b1690c to 0e5b4d6 Compare July 27, 2026 12:57

@SequeI SequeI left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm! thanks for the fix too

@SequeI
SequeI merged commit 65163c6 into nolabs-ai:main Jul 27, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support brokering credentials for CLIs that use a non-Bearer auth header and a localhost OIDC callback

3 participants