Skip to content

fix(sandbox): resolve secret placeholders inside HTTP Basic credentials (git-over-HTTPS 401) - #3

Closed
striderkein wants to merge 1 commit into
simount:fix/sandbox-dns-udp-acceptfrom
striderkein:fix/basic-auth-placeholder
Closed

fix(sandbox): resolve secret placeholders inside HTTP Basic credentials (git-over-HTTPS 401)#3
striderkein wants to merge 1 commit into
simount:fix/sandbox-dns-udp-acceptfrom
striderkein:fix/basic-auth-placeholder

Conversation

@striderkein

@striderkein striderkein commented Sep 8, 2026

Copy link
Copy Markdown

Summary

rewrite_header_value (crates/openshell-sandbox/src/secrets.rs) resolves openshell:resolve:env:* placeholders only when they appear as the whole header value or as plaintext <scheme> <placeholder>. git-over-HTTPS sends credentials as Authorization: Basic base64("<user>:<password>"), so a placeholder password (what gh auth git-credential returns) is invisible to the resolver, reaches GitHub verbatim, and every git fetch / git push gets 401 — while gh (which sends Authorization: token <placeholder>) works. GitHub's git smart-HTTP endpoint does not accept Bearer/token (verified: 401), so this cannot be worked around on the git side via http.extraHeader.

This decodes the Basic credential, resolves the password segment if it is a placeholder, and re-encodes. The username is left untouched. Basic values with no placeholder are left unmodified (returns None, same as before).

Adds base64 = { workspace = true } to the sandbox crate (already a workspace dep).

Verification

End-to-end on a live sandbox (autodev-pf-1, NemoClaw-on-AWS), release build of this branch installed as pid1:

check before after
git ls-remote https://github.com/simount/autodev-brain-pf.git HEAD (default gh auth git-credential helper) 401 HEAD resolved
vault git fetch origin 401 rc=0
workspace git fetch origin develop 401 rc=0
git push --dry-run origin HEAD:refs/heads/_probe 401 rc=0 (write auth OK, no write)
gh api user (token-scheme path) OK OK (unchanged)
OPENSHELL_DIRECT_TCP_ENDPOINTS (5432) open open (unchanged)

Two unit tests are included (resolves_placeholder_inside_basic_auth, leaves_basic_without_placeholder_untouched). Note: cargo test -p openshell-sandbox currently fails to compile the lib-test target due to pre-existing std::env::remove_var calls that are unsafe under edition 2024 (19 errors, unrelated to this change), so the new tests could not be executed in CI-equivalent form; the behavior is covered by the live verification above. Happy to fix the pre-existing unsafe-env test debt in a separate PR if wanted.

Context

Found while bringing up a second NemoClaw instance (simount/NemoClaw-on-AWS#207). The S3-distributed sandbox binary there predates both OPENSHELL_DIRECT_TCP_ENDPOINTS (e21966b) and this fix; rebuilding from this branch head resolves both.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KdE595yg6WVtZu324T5Ce3


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

`rewrite_header_value` only resolved `openshell:resolve:env:*` placeholders
when they appeared as the whole header value or as `<scheme> <placeholder>`
in plaintext. git-over-HTTPS sends credentials as
`Authorization: Basic base64("<user>:<password>")`, so a placeholder
password (e.g. what `gh auth git-credential` returns) was invisible to the
resolver, reached GitHub verbatim, and every git fetch/push got 401 — while
`gh` (which sends `token <placeholder>`) worked. GitHub's git endpoint does
not accept Bearer/token, so this cannot be worked around on the git side.

Decode the Basic credential, resolve the password segment if it is a
placeholder, and re-encode; leave the username untouched. Basic values that
contain no placeholder are left unmodified.

Adds `base64` (workspace dep) to the sandbox crate.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KdE595yg6WVtZu324T5Ce3
@striderkein

Copy link
Copy Markdown
Author

Closing in favor of the upstream implementation.

The Basic-auth credential resolution this PR adds by hand (~40 lines in secrets.rs) is already implemented, more comprehensively, in NVIDIA#708 ("extend L7 credential injection to query params, Basic auth, and URL paths", merged 2026-04-01, secrets.rs +1242 plus l7/rest.rs / l7/relay.rs / proxy.rs). Duplicating a slice of it on the fix/sandbox-dns-udp-accept branch is the wrong layer to fix this.

Root cause of the git-over-HTTPS 401 is that the deployed branch (fix/sandbox-dns-udp-accept, the source of the S3 sandbox binary) is based on an upstream snapshot from before NVIDIA#708 (diverged: behind 19). It has OPENSHELL_DIRECT_TCP_ENDPOINTS but not NVIDIA#708's Basic-auth injection, so neither the deployed branch nor kosaku's fix/basic-auth-credential-resolve branch (which references NVIDIA#708 but lacks ENDPOINTS) has both.

The right fix is a fork-maintenance task owned by @kosaku-sim: re-sync simount/OpenShell to an upstream that includes NVIDIA#708 and re-apply the ENDPOINTS / DNS / PTY patches on top, then rebuild the S3 binary. Tracked in simount/NemoClaw-on-AWS#207.

The hand-rolled patch remains only as a stopgap on autodev-pf-1 (backup binary kept) so git works there until the fork is re-synced. Personal fork striderkein/OpenShell will be deleted.

@striderkein striderkein closed this Sep 8, 2026
@striderkein
striderkein deleted the fix/basic-auth-placeholder branch September 8, 2026 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant