feat(sandbox): extend L7 credential injection — query params, Basic auth, URL paths

[johntmyers]

Problem Statement

The L7 proxy's SecretResolver injects credentials into HTTP headers but cannot rewrite query parameters, Basic auth tokens, or URL paths. Three concrete use cases are blocked:

1. Query parameter APIs (YouTube Data API, Google APIs) — authenticate via ?key=VALUE in the URL
2. Basic auth APIs (container registries, legacy REST) — require base64-encoded username:password in the Authorization: Basic header, with the credential embedded inside the encoded token
3. URL path APIs (Telegram Bot API) — embed the token in the URL path (/bot{TOKEN}/sendMessage)

NemoClaw is actively migrating service credentials to openshell provider create. Discord and Slack work today (header-based). Telegram is the first concrete blocker — the token is in the path, not a header. Query param and Basic auth gaps also block Google API and registry integrations.

Related

• feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631 — PR implementing query param rewriting + Basic auth encoding (from external contributor, to be absorbed here)
• feat(sandbox): L7 credential injection — query parameter rewriting and Basic auth encoding #630 — issue tracking query param + Basic auth gaps
• feat: L7 credential injection for non-inference providers #538 / feat(sandbox): L7 credential injection for non-inference providers #541 — prior proposals for broader credential injection (closed — too broad)

What Exists Today

SecretResolver (crates/openshell-sandbox/src/secrets.rs) replaces openshell:resolve:env:{KEY} placeholders in HTTP header values. The child process gets placeholder tokens in its env instead of real secrets. When the proxy intercepts an outbound request, rewrite_http_header_block iterates header lines and substitutes real values. The request line and body are passed through untouched.

Key Code Locations

Location	What It Does
crates/openshell-sandbox/src/secrets.rs:8-48	SecretResolver — placeholder map, resolve_placeholder(), rewrite_header_value()
crates/openshell-sandbox/src/secrets.rs:55-97	rewrite_http_header_block / rewrite_header_line — per-line header rewriting
crates/openshell-sandbox/src/l7/rest.rs:~152-196	relay_http_request_with_resolver — calls rewrite before upstream write
crates/openshell-sandbox/src/l7/relay.rs:~108-111	L7RequestInfo — req.target passed to OPA for path-based L7 policy eval
crates/openshell-sandbox/src/proxy.rs:~1594-1681	Forward proxy rewrite_forward_request — another rewrite call site

Recommendations

Tier 1: Do Now (no schema changes, follows existing patterns)

These extend the implicit placeholder resolution model already used for headers. The child process places openshell:resolve:env:* in the natural location, the proxy rewrites it. No policy YAML or proto changes needed.

1a. Query parameter rewriting
Resolve placeholders in URL query param values. PR #631 has a working implementation: rewrite_request_line parses METHOD URI HTTP/x.x, splits the URI at ?, iterates key=value pairs, resolves placeholder values, and percent-encodes per RFC 3986. Absorb this work.

1b. Basic auth decode/resolve/re-encode
When Authorization: Basic <base64> contains a placeholder in the decoded user:password string, decode → resolve → re-encode. PR #631 has a working implementation via rewrite_basic_auth_token. Absorb this work.

1c. URL path placeholder resolution
Extend rewrite_request_line (from 1a) to also scan the path component for openshell:resolve:env:* and resolve. This is the Telegram blocker. The insertion point is clear — rewrite_request_line already parses the URI; add a rewrite_uri_path step before the query param step.

Security hardening specific to path rewriting:

• Fail-closed: if a path placeholder can't be resolved, reject the request (don't forward the raw placeholder — it leaks into server logs and error pages, unlike header placeholders which just cause a 401)
• Sanitize resolved values: reject or escape credential values containing ../, null bytes, or query delimiters (?, #) to prevent path traversal
• Percent-encode resolved values per RFC 3986 path segment rules (different character set than query params)

Tier 2: Defer (requires architectural changes)

These patterns either lack immediate demand signal or require changes to the proxy's streaming relay model.

Request body rewriting — Some webhook APIs put credentials in JSON bodies. This requires buffering the entire body (today it streams via relay_fixed/relay_chunked), rewriting, recalculating Content-Length, and re-chunking. Fundamentally changes the relay model. No current demand signal.

Cookie injection — Cookie headers have key=value; key2=value2 structure that rewrite_header_value doesn't parse. Low demand. Could be added later without architectural changes if needed.

Explicit credential_injection policy config — Adding a credential_injection block to NetworkEndpoint in proto/YAML (as proposed in #538/#541). This was rejected for being too broad. May be worth revisiting if/when body rewriting or more complex injection patterns are needed, but the implicit placeholder model covers Tier 1 cleanly.

Composite auth schemes — HMAC signing, AWS Signature V4, OAuth token refresh. These require stateful computation, not just string substitution. Out of scope for the placeholder model entirely.

Open Questions

1. OPA evaluation order for path rewriting — L7 Rego evaluates request.path as-is from the client. If the path contains a placeholder, OPA sees /botopenshell:resolve:env:TOKEN/sendMessage. Should path rewriting happen before or after OPA eval? Before means OPA sees the real secret (logging risk). After means L7 rules must match placeholder patterns. Recommend: rewrite before OPA, but strip the resolved token from log output.

2. Fail-closed consistency — Header placeholder leakage fails open today (placeholder forwarded, upstream returns 401). Path rewriting should fail closed. Should we also make header/query rewriting fail-closed for consistency, or accept the divergence?

3. PR feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631 disposition — The implementation in PR feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631 covers 1a and 1b. Options: (a) absorb the diff into a new branch with 1c added, (b) merge feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631 as-is and follow up with 1c. Recommend (a) for a single coherent PR.

Scope Assessment

• Complexity: Low-Medium (Tier 1 only)
• Confidence: High — clear insertion points, follows existing patterns
• Estimated files to change: 3-4 (secrets.rs, rest.rs, Cargo.toml, tests)
• Issue type: feat

Test Considerations

• Unit tests for path placeholder detection, resolution, and percent-encoding (RFC 3986 path segment rules)
• Security tests: path traversal via ../, null bytes, query delimiters in credential values
• Fail-closed test: unresolvable path placeholder → request rejected
• Integration test with mock Telegram-style /bot{TOKEN}/method endpoint
• Absorb PR feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631's 11 existing tests for query param and Basic auth
• Regression: existing header rewriting still works

---

Created by spike investigation. Absorbs PR #631 scope. Use build-from-issue to implement.

[johntmyers]

🏗️ build-plan

Implementation Plan

Issue type: feat
Complexity: Medium
Confidence: High — clear insertion points, absorbs proven code from PR #631

Summary

Extend SecretResolver to resolve openshell:resolve:env:* placeholders in three new locations: URL query parameters, Basic auth tokens, and URL path segments. Absorb working code from PR #631 for the first two. Add path rewriting for Telegram-style APIs. Change all placeholder rewriting to fail-closed. Rewrite request targets before OPA evaluation, feeding OPA a redacted path (secrets replaced with [CREDENTIAL]) while forwarding the resolved path upstream. Validate all resolved secret values for control characters (CRLF, null) to prevent header injection.

Scope

• crates/openshell-sandbox/Cargo.toml: Add base64 dependency
• crates/openshell-sandbox/src/secrets.rs: Core rewriting logic — secret validation, query params, Basic auth, path segments, fail-closed, redaction types
• crates/openshell-sandbox/src/l7/relay.rs: Pre-rewrite target before OPA eval, feed OPA redacted path, redact logs
• crates/openshell-sandbox/src/l7/rest.rs: Handle Result from rewrite_http_header_block, redact target in deny responses
• crates/openshell-sandbox/src/proxy.rs: Handle Result from rewrite_forward_request

Explicitly excluded: PR #631's build.rs change (prefers system PROTOC over bundled — breaks CI). The release-fork.yml workflow file from PR #631 (fork-specific).

Security Findings Incorporated

This plan incorporates findings from a principal-engineer security review using OWASP/CWE frameworks:

#	Severity	Finding	Addressed In
F1	Critical	CWE-113: CRLF injection via resolved credential values	Step 2
F2	Critical	OWASP A01: OPA must see redacted path, not real secrets	Step 5
F3	High	CWE-22: Path traversal validation must cover encoded variants, \, /	Step 3
F4	High	CWE-532: All log sites and deny responses must use redacted targets	Step 5
F5	High	CWE-116: Fail-closed scan must check percent-decoded form	Step 4
F7	Medium	CWE-20: Placeholder extraction grammar must be explicit	Step 3
F8	Medium	OWASP A01: Credentials resolve regardless of destination host	Docs
F6	Medium	CWE-444: Test Content-Length correctness after path rewrite	Step 7
F10	Medium	OWASP A02: Base64 edge cases (non-UTF8, multiple :, padding)	Step 3

Implementation Steps

Step 1: Add base64 dependency
Add base64 = { workspace = true } to crates/openshell-sandbox/Cargo.toml under dependencies.

Step 2: Resolved secret validation (F1 — Critical)
Add validation at the resolve_placeholder level in SecretResolver, so all rewrite paths (headers, query, path, Basic auth) are protected automatically:

fn validate_resolved_secret(value: &str) -> Result<&str, &'static str> {
    if value.bytes().any(|b| b == b'\r' || b == b'\n' || b == b'\0') {
        return Err("resolved secret contains prohibited control characters");
    }
    Ok(value)
}

Modify resolve_placeholder to call validate_resolved_secret and return None (triggering fail-closed downstream) if validation fails, plus emit a tracing::warn! noting the location without revealing the value. This fixes a pre-existing vulnerability in the current header rewriting code, not just the new work.

Add tests:

• resolve_placeholder_rejects_crlf — secret containing \r\n → None
• resolve_placeholder_rejects_null — secret containing \0 → None
• resolve_placeholder_accepts_normal_values — standard API keys pass

Step 3: Absorb PR #631 + add path rewriting
Manually apply the relevant code from PR #631, then add path rewriting on top:

3a. Percent encoding/decoding helpers:

• percent_decode(input) -> String
• percent_encode_query(input) -> String — RFC 3986 query value encoding
• percent_encode_path_segment(input) -> String — RFC 3986 §3.3 path segment encoding (different allowed character set: must encode ?, #, /; must NOT encode :, @)

3b. Basic auth (from PR #631 + F10 hardening):

• SecretResolver::rewrite_basic_auth_token(encoded) -> Option<String> — base64 decode with general_purpose::STANDARD, handle decode failure gracefully (return None, no rewrite), from_utf8 failure returns None, split decoded on first : only (handles user:pass:extra), resolve placeholders in both halves, re-encode
• Extend rewrite_header_value to detect Basic prefix (case-insensitive) and delegate to rewrite_basic_auth_token

3c. Query param rewriting (from PR #631):

• rewrite_uri_query_params(query, resolver) -> Option<String> — split on &, for each key=value percent-decode value, resolve placeholder, percent-encode resolved secret, reconstruct

3d. URL path rewriting (new work + F3, F7 hardening):

• validate_credential_for_path(value) -> Result<(), String> — operate on the decoded form; reject ../, ..\\, \0, \r, \n, /, \, ?, #
• Placeholder extraction grammar: openshell:resolve:env:[A-Za-z_][A-Za-z0-9_]* — use this regex-like boundary to determine where each placeholder ends within a concatenated path segment (handles Telegram /bot{TOKEN}/method and prevents ambiguous/greedy matching with multiple adjacent placeholders)
• rewrite_uri_path(path, resolver) -> Result<Option<(String, String)>, Error> — split on /, percent-decode each segment, scan for PLACEHOLDER_PREFIX, extract key using grammar boundary, resolve, validate, percent-encode for path segment context; return (resolved_path, redacted_path) where redacted replaces each resolved value with [CREDENTIAL]

3e. Request line integration:

• rewrite_request_line(line, resolver) -> RewriteLineResult — parse METHOD URI HTTP/version, split URI at ?, call rewrite_uri_path on path, call rewrite_uri_query_params on query, reassemble; return struct with resolved line and redacted target
• Modify rewrite_http_header_block to call rewrite_request_line on the request line (currently passes it through verbatim)

3f. Tests (21 new):

• 11 absorbed from PR feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631 (query params, Basic auth, percent encoding)
• rewrite_path_single_segment_placeholder
• rewrite_path_telegram_style_concatenated — /botopenshell:resolve:env:TOKEN/sendMessage
• rewrite_path_multiple_placeholders_in_separate_segments
• rewrite_path_no_placeholders_unchanged
• rewrite_path_preserves_query_params
• rewrite_path_credential_traversal_rejected — value containing ../
• rewrite_path_credential_backslash_rejected — value containing \
• rewrite_path_credential_slash_rejected — value containing /
• rewrite_path_credential_null_rejected
• rewrite_path_percent_encodes_special_chars

Step 4: Fail-closed behavior (F5 hardening)
Change all placeholder rewriting to reject requests containing unresolved placeholders:

• Define result/error types:

  pub(crate) struct RewriteResult {
      pub rewritten: Vec<u8>,
      pub redacted_target: Option<String>,
  }
  
  pub(crate) struct UnresolvedPlaceholderError {
      pub location: String, // "header", "query_param", "path"
  }

  Error Display does NOT include the env var name or placeholder — only the location.

• Change rewrite_http_header_block signature to -> Result<RewriteResult, UnresolvedPlaceholderError>

• After all rewriting, scan for remaining PLACEHOLDER_PREFIX in:

  • The rewritten header bytes (raw form)
  • The percent-decoded form of the rewritten request line (catches encoded placeholder bypass per F5)

• When resolver is None, pass through without scanning

• Update call site in relay_http_request_with_resolver (rest.rs): propagate error, caller sends 500 to child

• Update rewrite_forward_request in proxy.rs to return Result, caller sends 500 Internal Server Error on failure

Tests (6 new):

• unresolved_header_placeholder_returns_error
• unresolved_query_param_returns_error
• unresolved_path_placeholder_returns_error
• percent_encoded_placeholder_in_path_caught — %6F%70%65%6E%73%68%65%6C%6C%3A... form detected
• all_resolved_succeeds
• no_resolver_passes_through_without_scanning

Step 5: Rewrite-before-OPA + redaction (F2, F4)
Move credential resolution of the request target to BEFORE OPA evaluation. OPA receives the redacted path (structure-preserving, secrets replaced with [CREDENTIAL]). The resolved path goes only to the upstream write.

• Add rewrite_target_for_eval to secrets.rs:

  pub(crate) struct RewriteTargetResult {
      pub resolved: String,   // real secrets — for upstream only
      pub redacted: String,   // [CREDENTIAL] — for OPA + logs
  }
  
  pub(crate) fn rewrite_target_for_eval(
      target: &str,
      resolver: &SecretResolver,
  ) -> Result<RewriteTargetResult, UnresolvedPlaceholderError>

• In relay_with_inspection (relay.rs): call rewrite_target_for_eval before building L7RequestInfo. Feed redacted into L7RequestInfo.target for OPA evaluation. On error, send deny response and return.

• In relay_passthrough_with_credentials (relay.rs): same pre-rewrite for logging redaction.

• In handle_forward_proxy (proxy.rs): apply rewrite_target_for_eval to path before building L7RequestInfo for the forward L7 eval path.

• Redact ALL log statements and deny responses:

  • relay.rs:~129 l7_target: use redacted
  • relay.rs:~272 path: use redacted
  • rest.rs:~208 deny response body (req.action + req.target): use redacted
  • Forward proxy logs (proxy.rs:~1780, ~1866, ~1998): safe — path from absolute URI parse, not child-constructed. No change needed.

• The full rewrite_http_header_block still runs after OPA in relay_http_request_with_resolver — it rewrites headers and re-resolves the request line (idempotent for already-resolved targets). Fail-closed scan catches any remaining unresolved placeholders.

Tests (4 new):

• redacted_target_replaces_path_secrets_with_credential_marker
• redacted_target_replaces_query_secrets_with_credential_marker
• redacted_target_preserves_non_secret_segments
• rewrite_target_for_eval_roundtrip

Step 6: Integration verification
Run full crate test suite. Specific verification:

• No double-rewriting artifacts (pre-rewrite for OPA + full rewrite for upstream are idempotent)
• Request with path placeholder + body forwards correct body bytes (F6 — Content-Length boundary correctness)
• Existing header rewriting tests still pass
• mise run pre-commit passes

Test Plan

• Unit tests (34 new): All in crates/openshell-sandbox/src/secrets.rs — 3 secret validation (F1), 11 absorbed from PR feat(sandbox): L7 credential injection — query param rewriting and Basic auth encoding #631, 10 path rewriting, 6 fail-closed, 4 redaction
• Integration tests (4 new): In crates/openshell-sandbox/src/l7/rest.rs — relay with query param credentials, Basic auth credentials, path credentials, fail-closed 500 response + body correctness (F6)
• E2E tests: N/A — no changes under e2e/

Risks

1. Substring placeholder matching in paths — Telegram's /bot{TOKEN}/method means the placeholder is concatenated with literal text. The explicit grammar (openshell:resolve:env:[A-Za-z_][A-Za-z0-9_]*) provides a clear extraction boundary, but the implementation needs careful iterative scanning with re-scan after each replacement to handle multiple adjacent placeholders.

2. Double base64 edge case — If a child sends a non-placeholder Authorization: Basic token that isn't valid base64, rewrite_basic_auth_token returns None (no rewrite, pass through). Decode failure is not an error.

3. Credential exfiltration via host routing (F8) — The resolver resolves all placeholders regardless of destination host. An agent with OPA-allowed access to attacker.com could place a placeholder in a query param and exfiltrate the resolved secret. OPA host restrictions are the defense. Document this as a known limitation. Per-credential host binding is a future enhancement.

Documentation Impact

• architecture/sandbox.md: Update credential injection section — document query param, Basic auth, and path rewriting; fail-closed behavior; redaction model; the host-binding limitation (F8)

---

Revision 2 — incorporated security review findings (F1-F10: CRLF validation, OPA redaction, path traversal hardening, encoded placeholder scan, placeholder grammar, Base64 edge cases, Content-Length test, host-binding documentation)
Revision 1 — initial plan

[htekdev]

Im working on this and will cleanup my PRs today. Its been hard to valid but I am setup now so testing validating now. Unless you have plans other wise.

[johntmyers]

We will issue a new PR. We cannot run the full test suite against forked PRs.