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

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:

Query parameter APIs (YouTube Data API, Google APIs) — authenticate via ?key=VALUE in the URL
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
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.

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

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.
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?
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.

🏗️ 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

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.
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.
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

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.

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

Live Sandbox Verification of PR #631 Changes

Tested the query param + Basic auth injection from PR #631 end-to-end in a live sandbox. Built the sandbox binary with the #631 changes, docker cp'd into the running cluster, and validated against real APIs.

Results

Injection Type	Target	Result
`Bearer $PLACEHOLDER`	GitHub `/user`	✅ 200
`token $PLACEHOLDER`	GitHub `/user`	✅ 200
`Basic base64(user:$PLACEHOLDER)`	GitHub `/user`	✅ 200
`?key=$PLACEHOLDER` (query param)	YouTube Data API v3	✅ Real search results

All four paths confirmed working. The proxy correctly resolves placeholders in headers, base64-decoded Basic auth, and URL query parameters.

Copilot CLI Blocker

Also discovered during this session: the Copilot CLI validates token format client-side before making any HTTP request. The openshell:resolve:env:* placeholder gets rejected before the proxy can inject. Filed upstream: github/copilot-cli#2431. The missing Copilot CLI enum variant is tracked in #707.

Related Items Created During This Session

Item	Description
Issue #707	Missing Copilot CLI enum variant (fix: `htekdev:fix/copilot-cli-enum`)
github/copilot-cli#2431	Copilot CLI client-side token validation blocks proxy injection
PR #631 comment	Detailed test results with debug log traces
Issue #630 comment	Verification summary

feat(sandbox): extend L7 credential injection to query params, Basic …

🏗️ build-from-issue-agent

Implementation Complete

PR: #708

What was built

Extended L7 credential injection to resolve placeholders in URL query parameters, Basic auth tokens, and URL path segments. Changed all rewriting to fail-closed. Added secret value validation (CWE-113) and path credential validation (CWE-22). OPA evaluates against redacted paths; real secrets appear only in upstream connections.

Tests

Unit: 34 tests added (secrets.rs)
Integration: existing relay tests updated for new Result types
E2E: N/A

Docs updated

architecture/sandbox.md: credential injection pipeline, validation, fail-closed, redaction
architecture/sandbox-providers.md: proxy-time resolution for all placement types
docs/sandboxes/manage-providers.md: user-facing docs for supported injection locations

The issue will auto-close when the PR is merged.

feat(sandbox): extend L7 credential injection to query params, Basic …

chore: upstream merge 2026-04-08 (Wave 3) (#5)

Navigation Menu

Repository navigation

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

Problem Statement

Related

What Exists Today

Key Code Locations

Recommendations

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

Tier 2: Defer (requires architectural changes)

Open Questions

Scope Assessment

Test Considerations

Implementation Plan

Summary

Scope

Security Findings Incorporated

Implementation Steps

Test Plan

Risks

Documentation Impact

Live Sandbox Verification of PR #631 Changes

Results

Copilot CLI Blocker

Related Items Created During This Session

Implementation Complete

What was built

Tests

Docs updated

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Problem Statement

Related

What Exists Today

Key Code Locations

Recommendations

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

Tier 2: Defer (requires architectural changes)

Open Questions

Scope Assessment

Test Considerations

Activity

johntmyers commented on Mar 30, 2026

Implementation Plan

Summary

Scope

Security Findings Incorporated

Implementation Steps

Test Plan

Risks

Documentation Impact

htekdev commented on Mar 31, 2026

johntmyers commented on Mar 31, 2026

htekdev commented on Mar 31, 2026

Live Sandbox Verification of PR #631 Changes

Results

Copilot CLI Blocker

Related Items Created During This Session

johntmyers commented on Mar 31, 2026

Implementation Complete

What was built

Tests

Docs updated

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions