Skip to content

Commit b942fa5

Browse files
committed
feat(sandbox): extend L7 credential injection to query params, Basic auth, and URL paths
Closes #689 Extend SecretResolver to resolve openshell:resolve:env:* placeholders in URL query parameters, Basic auth tokens, and URL path segments. Absorbs working code from PR #631 for query param and Basic auth support. Adds path rewriting for Telegram-style APIs (/bot{TOKEN}/method). Changes all placeholder rewriting to fail-closed: unresolved placeholders cause HTTP 500 instead of forwarding raw placeholder strings. Validates resolved secret values for CRLF/null injection (CWE-113). Validates path credentials for traversal sequences (CWE-22). Rewrites request targets before OPA L7 policy evaluation. OPA receives a redacted path with [CREDENTIAL] markers. Real secrets appear only in the upstream connection. All log statements use redacted targets.
1 parent 2538bea commit b942fa5

File tree

9 files changed

+1564
-79
lines changed

9 files changed

+1564
-79
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

architecture/sandbox-providers.md

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -305,18 +305,31 @@ start from `env_clear()`, so the handshake secret is not present there.
305305

306306
### Proxy-Time Secret Resolution
307307

308-
When a sandboxed tool uses one of these placeholder env vars to populate an outbound HTTP
309-
header (for example `Authorization: Bearer openshell:resolve:env:ANTHROPIC_API_KEY`), the
310-
sandbox proxy rewrites the placeholder to the real secret value immediately before the
311-
request is forwarded upstream.
312-
313-
This applies to:
314-
315-
- forward-proxy HTTP requests, and
316-
- L7-inspected REST requests inside CONNECT tunnels.
308+
When a sandboxed tool uses one of these placeholder env vars in an outbound HTTP request,
309+
the sandbox proxy rewrites the placeholder to the real secret value immediately before the
310+
request is forwarded upstream. Placeholders are resolved in four locations:
311+
312+
- **HTTP header values** — exact match (`x-api-key: openshell:resolve:env:KEY`), prefixed
313+
match (`Authorization: Bearer openshell:resolve:env:KEY`), and Base64-decoded Basic auth
314+
tokens (`Authorization: Basic <base64(user:openshell:resolve:env:PASS)>`)
315+
- **URL query parameters** — for APIs that authenticate via query string
316+
(e.g., `?key=openshell:resolve:env:YOUTUBE_API_KEY`)
317+
- **URL path segments** — for APIs that embed tokens in the URL path
318+
(e.g., `/bot<placeholder>/sendMessage` for Telegram Bot API)
319+
320+
This applies to forward-proxy HTTP requests, L7-inspected REST requests inside CONNECT
321+
tunnels, and credential-injection-only passthrough relays on TLS-terminated connections.
322+
323+
All rewriting fails closed: if any `openshell:resolve:env:*` placeholder is detected but
324+
cannot be resolved, the proxy rejects the request with HTTP 500 instead of forwarding the
325+
raw placeholder upstream. Resolved secret values are validated for prohibited control
326+
characters (CR, LF, null byte) to prevent header injection (CWE-113). Path segment
327+
credentials are additionally validated to reject traversal sequences, path separators, and
328+
URI delimiters (CWE-22).
317329

318330
The real secret value remains in supervisor memory only; it is not re-injected into the
319-
child process environment.
331+
child process environment. See [Credential injection](sandbox.md#credential-injection) for
332+
the full implementation details, encoding rules, and security properties.
320333

321334
### End-to-End Flow
322335

architecture/sandbox.md

Lines changed: 135 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ All paths are relative to `crates/openshell-sandbox/src/`.
3333
| `l7/relay.rs` | Protocol-aware bidirectional relay with per-request OPA evaluation, credential-injection-only passthrough relay |
3434
| `l7/rest.rs` | HTTP/1.1 request/response parsing, body framing (Content-Length, chunked), deny response generation |
3535
| `l7/provider.rs` | `L7Provider` trait and `L7Request`/`BodyLength` types |
36+
| `secrets.rs` | `SecretResolver` credential placeholder system — placeholder generation, multi-location rewriting (headers, query params, path segments, Basic auth), fail-closed scanning, secret validation, percent-encoding |
3637

3738
## Startup and Orchestration
3839

@@ -824,11 +825,13 @@ When `Router::proxy_with_candidates()` returns an error, `router_error_to_http()
824825

825826
| `RouterError` variant | HTTP status | Response body |
826827
|----------------------|-------------|---------------|
827-
| `RouteNotFound(hint)` | `400` | `no route configured for route '{hint}'` |
828-
| `NoCompatibleRoute(protocol)` | `400` | `no compatible route for source protocol '{protocol}'` |
829-
| `Unauthorized(msg)` | `401` | `{msg}` |
830-
| `UpstreamUnavailable(msg)` | `503` | `{msg}` |
831-
| `UpstreamProtocol(msg)` / `Internal(msg)` | `502` | `{msg}` |
828+
| `RouteNotFound(_)` | `400` | `no inference route configured` |
829+
| `NoCompatibleRoute(_)` | `400` | `no compatible inference route available` |
830+
| `Unauthorized(_)` | `401` | `unauthorized` |
831+
| `UpstreamUnavailable(_)` | `503` | `inference service unavailable` |
832+
| `UpstreamProtocol(_)` / `Internal(_)` | `502` | `inference service error` |
833+
834+
Response messages are generic — internal details (upstream URLs, hostnames, TLS errors, route hints) are never exposed to the sandboxed process. Full error context is logged server-side at `warn` level.
832835

833836
### Inference routing context
834837

@@ -1027,20 +1030,131 @@ TLS termination is automatic. The proxy peeks the first bytes of every CONNECT t
10271030

10281031
System CA bundles are searched at well-known paths: `/etc/ssl/certs/ca-certificates.crt` (Debian/Ubuntu), `/etc/pki/tls/certs/ca-bundle.crt` (RHEL), `/etc/ssl/ca-bundle.pem` (openSUSE), `/etc/ssl/cert.pem` (Alpine/macOS).
10291032

1030-
### Credential-injection-only relay
1033+
### Credential injection
1034+
1035+
**Files:** `crates/openshell-sandbox/src/secrets.rs`, `crates/openshell-sandbox/src/l7/relay.rs`, `crates/openshell-sandbox/src/l7/rest.rs`, `crates/openshell-sandbox/src/proxy.rs`
1036+
1037+
The sandbox proxy resolves `openshell:resolve:env:*` credential placeholders in outbound HTTP requests. The `SecretResolver` holds a supervisor-only map from placeholder strings to real secret values, constructed at startup from the provider environment. Child processes only see placeholder values in their environment; the proxy rewrites them to real secrets immediately before forwarding upstream.
1038+
1039+
#### `SecretResolver`
1040+
1041+
```rust
1042+
pub(crate) struct SecretResolver {
1043+
by_placeholder: HashMap<String, String>,
1044+
}
1045+
```
1046+
1047+
`SecretResolver::from_provider_env()` splits the provider environment into two maps: a child-visible map with placeholder values (`openshell:resolve:env:ANTHROPIC_API_KEY`) and a supervisor-only resolver map (`{"openshell:resolve:env:ANTHROPIC_API_KEY": "sk-real-key"}`). The placeholder grammar is `openshell:resolve:env:[A-Za-z_][A-Za-z0-9_]*`.
1048+
1049+
#### Credential placement locations
1050+
1051+
The resolver rewrites placeholders in four locations within HTTP requests:
1052+
1053+
| Location | Example | Encoding | Implementation |
1054+
|----------|---------|----------|----------------|
1055+
| Header value (exact) | `x-api-key: openshell:resolve:env:KEY` | None (raw replacement) | `rewrite_header_value()` |
1056+
| Header value (prefixed) | `Authorization: Bearer openshell:resolve:env:KEY` | None (prefix preserved) | `rewrite_header_value()` |
1057+
| Basic auth token | `Authorization: Basic <base64(user:openshell:resolve:env:PASS)>` | Base64 decode → resolve → re-encode | `rewrite_basic_auth_token()` |
1058+
| URL query parameter | `?key=openshell:resolve:env:KEY` | Percent-decode → resolve → percent-encode (RFC 3986 unreserved) | `rewrite_uri_query_params()` |
1059+
| URL path segment | `/bot<placeholder>/sendMessage` | Percent-decode → resolve → validate → percent-encode (RFC 3986 pchar) | `rewrite_uri_path()``rewrite_path_segment()` |
1060+
1061+
**Header values**: Direct match replaces the entire value. Prefixed match (e.g., `Bearer <placeholder>`) splits on whitespace, resolves the placeholder portion, and reassembles. Basic auth match detects `Authorization: Basic <base64>`, decodes the Base64 content, resolves any placeholders in the decoded `user:password` string, and re-encodes.
1062+
1063+
**Query parameters**: Each `key=value` pair is checked. Values are percent-decoded before resolution and percent-encoded after (RFC 3986 Section 2.3 unreserved characters preserved: `ALPHA / DIGIT / "-" / "." / "_" / "~"`).
1064+
1065+
**Path segments**: Handles substring matching for APIs that embed tokens within path segments (e.g., Telegram's `/bot{TOKEN}/sendMessage`). Each segment is percent-decoded, scanned for placeholder boundaries using the env var key grammar (`[A-Za-z_][A-Za-z0-9_]*`), resolved, validated for path safety, and percent-encoded per RFC 3986 Section 3.3 pchar rules (`unreserved / sub-delims / ":" / "@"`).
1066+
1067+
#### Path credential validation (CWE-22)
1068+
1069+
Resolved credential values destined for URL path segments are validated by `validate_credential_for_path()` before insertion. The following values are rejected:
1070+
1071+
| Pattern | Rejection reason |
1072+
|---------|-----------------|
1073+
| `../`, `..\\`, `..` | Path traversal sequence |
1074+
| `/`, `\` | Path separator |
1075+
| `\0`, `\r`, `\n` | Control character |
1076+
| `?`, `#` | URI delimiter |
1077+
1078+
Rejection causes the request to fail closed (HTTP 500).
1079+
1080+
#### Secret value validation (CWE-113)
1081+
1082+
All resolved credential values are validated at the `resolve_placeholder()` level for prohibited control characters: CR (`\r`), LF (`\n`), and null byte (`\0`). This prevents HTTP header injection via malicious credential values. The validation applies to all placement locations automatically — header values, query parameters, and path segments all pass through `resolve_placeholder()`.
1083+
1084+
#### Fail-closed behavior
1085+
1086+
All placeholder rewriting fails closed. If any `openshell:resolve:env:*` placeholder is detected in the request but cannot be resolved, the proxy rejects the request with HTTP 500 instead of forwarding the raw placeholder to the upstream. The fail-closed mechanism operates at two levels:
1087+
1088+
1. **Per-location**: Each rewrite function (`rewrite_uri_query_params`, `rewrite_path_segment`, `rewrite_header_line`) returns an `UnresolvedPlaceholderError` when a placeholder is detected but the resolver has no mapping for it.
1089+
1090+
2. **Final scan**: After all rewriting completes, `rewrite_http_header_block()` scans the output for any remaining `openshell:resolve:env:` tokens. It also checks the percent-decoded form of the request line to catch encoded placeholder bypass attempts (e.g., `openshell%3Aresolve%3Aenv%3AUNKNOWN`).
1091+
1092+
```rust
1093+
pub(crate) struct UnresolvedPlaceholderError {
1094+
pub location: &'static str, // "header", "query_param", "path"
1095+
}
1096+
```
1097+
1098+
#### Rewrite-before-OPA with redaction
1099+
1100+
When L7 inspection is active, credential placeholders in the request target (path + query) are resolved BEFORE OPA L7 policy evaluation. This is implemented in `relay_with_inspection()` and `relay_passthrough_with_credentials()` in `l7/relay.rs`:
1101+
1102+
1. `rewrite_target_for_eval()` resolves the request target, producing two strings:
1103+
- **Resolved**: real secrets inserted — used only for the upstream connection
1104+
- **Redacted**: `[CREDENTIAL]` markers in place of secrets — used for OPA input and logs
1105+
1106+
2. OPA `evaluate_l7_request()` receives the redacted path in `request.path`, so policy rules never see real credential values.
1107+
1108+
3. All log statements (`L7_REQUEST`, `HTTP_REQUEST`) use the redacted target. Real credential values never appear in logs.
1109+
1110+
4. The resolved path (with real secrets) goes only to the upstream via `relay_http_request_with_resolver()`.
1111+
1112+
```rust
1113+
pub(crate) struct RewriteTargetResult {
1114+
pub resolved: String, // for upstream forwarding only
1115+
pub redacted: String, // for OPA + logs
1116+
}
1117+
```
1118+
1119+
If credential resolution fails on the request target, the relay returns HTTP 500 and closes the connection.
1120+
1121+
#### Credential-injection-only relay
10311122

10321123
**File:** `crates/openshell-sandbox/src/l7/relay.rs` (`relay_passthrough_with_credentials()`)
10331124

1034-
When TLS is auto-terminated but no L7 policy (`protocol` + `access`/`rules`) is configured on the endpoint, the proxy enters a passthrough mode that still provides value: it parses HTTP requests minimally to rewrite credential placeholders (via `SecretResolver`) and logs each request for observability. This relay:
1125+
When TLS is auto-terminated but no L7 policy (`protocol` + `access`/`rules`) is configured on the endpoint, the proxy enters a passthrough mode that still provides credential injection and observability. This relay:
10351126

10361127
1. Reads each HTTP request from the client via `RestProvider::parse_request()`
1037-
2. Logs the request method, path, host, and port at `info!()` level (tagged `"HTTP relay (credential injection)"`)
1038-
3. Forwards the request to upstream via `relay_http_request_with_resolver()`, which rewrites headers containing `openshell:resolve:env:*` placeholders with actual provider credential values
1039-
4. Relays the upstream response back to the client
1040-
5. Loops for HTTP keep-alive; exits on client close or non-reusable response
1128+
2. Resolves and redacts the request target via `rewrite_target_for_eval()` (for log safety)
1129+
3. Logs the request method, redacted path, host, and port at `info!()` level (tagged `HTTP_REQUEST`)
1130+
4. Forwards the request to upstream via `relay_http_request_with_resolver()`, which rewrites all credential placeholders in headers, query parameters, path segments, and Basic auth tokens
1131+
5. Relays the upstream response back to the client
1132+
6. Loops for HTTP keep-alive; exits on client close or non-reusable response
10411133

10421134
This enables credential injection on all HTTPS endpoints automatically, without requiring the policy author to add `protocol: rest` and `access: full` just to get credentials injected.
10431135

1136+
#### Known limitation: host-binding
1137+
1138+
The resolver resolves all placeholders regardless of destination host. If an agent has OPA-allowed access to an attacker-controlled host, it could construct a URL containing a placeholder and exfiltrate the resolved credential value to that host. OPA host restrictions are the defense — only endpoints explicitly allowed by policy receive traffic. Per-credential host binding (restricting which credentials resolve for which destination hosts) is not implemented.
1139+
1140+
#### Data flow
1141+
1142+
```mermaid
1143+
sequenceDiagram
1144+
participant A as Agent Process
1145+
participant P as Proxy (SecretResolver)
1146+
participant O as OPA Engine
1147+
participant U as Upstream API
1148+
1149+
A->>P: GET /bot<placeholder>/send?key=<placeholder> HTTP/1.1<br/>Authorization: Bearer <placeholder>
1150+
P->>P: rewrite_target_for_eval(target)<br/>→ resolved: /bot{secret}/send?key={secret}<br/>→ redacted: /bot[CREDENTIAL]/send?key=[CREDENTIAL]
1151+
P->>O: evaluate_l7_request(redacted path)
1152+
O-->>P: allow
1153+
P->>P: rewrite_http_header_block(headers)<br/>→ resolve header placeholders<br/>→ resolve query param placeholders<br/>→ resolve path segment placeholders<br/>→ fail-closed scan
1154+
P->>U: GET /bot{secret}/send?key={secret} HTTP/1.1<br/>Authorization: Bearer {secret}
1155+
Note over P: Logs use redacted path only
1156+
```
1157+
10441158
### REST protocol provider
10451159

10461160
**File:** `crates/openshell-sandbox/src/l7/rest.rs`
@@ -1060,11 +1174,12 @@ Implements `L7Provider` for HTTP/1.1:
10601174
`relay_with_inspection()` in `crates/openshell-sandbox/src/l7/relay.rs` is the main relay loop:
10611175

10621176
1. Parse one HTTP request from client via the provider
1063-
2. Build L7 input JSON with `request.method`, `request.path`, `request.query_params`, plus the CONNECT-level context (host, port, binary, ancestors, cmdline)
1064-
3. Evaluate `data.openshell.sandbox.allow_request` and `data.openshell.sandbox.request_deny_reason`
1065-
4. Log the L7 decision (tagged `L7_REQUEST`)
1066-
5. If allowed (or audit mode): relay request to upstream and response back to client, then loop
1067-
6. If denied in enforce mode: send 403 and close the connection
1177+
2. Resolve credential placeholders in the request target via `rewrite_target_for_eval()`. OPA receives the redacted path (`[CREDENTIAL]` markers); the resolved path goes only to upstream. If resolution fails, return HTTP 500 and close the connection.
1178+
3. Build L7 input JSON with `request.method`, the **redacted** `request.path`, `request.query_params`, plus the CONNECT-level context (host, port, binary, ancestors, cmdline)
1179+
4. Evaluate `data.openshell.sandbox.allow_request` and `data.openshell.sandbox.request_deny_reason`
1180+
5. Log the L7 decision (tagged `L7_REQUEST`) using the redacted target — real credential values never appear in logs
1181+
6. If allowed (or audit mode): relay request to upstream via `relay_http_request_with_resolver()` (which rewrites all remaining credential placeholders in headers, query parameters, path segments, and Basic auth tokens) and relay the response back to client, then loop
1182+
7. If denied in enforce mode: send 403 (using redacted target in the response body) and close the connection
10681183

10691184
## Process Identity
10701185

@@ -1317,6 +1432,10 @@ The sandbox uses `miette` for error reporting and `thiserror` for typed errors.
13171432
| Log push gRPC stream breaks | Push loop exits, flushes remaining batch |
13181433
| Proxy accept error | Log + break accept loop |
13191434
| Benign connection close (EOF, reset, pipe) | Debug level (not visible to user by default) |
1435+
| Credential injection: unresolved placeholder detected | HTTP 500, connection closed (fail-closed) |
1436+
| Credential injection: resolved value contains CR/LF/null | Placeholder treated as unresolvable, fail-closed |
1437+
| Credential injection: path credential contains traversal/separator | HTTP 500, connection closed (fail-closed) |
1438+
| Credential injection: percent-encoded placeholder bypass attempt | HTTP 500, connection closed (fail-closed) |
13201439
| L7 parse error | Close the connection |
13211440
| SSH server failure | Async task error logged, main process unaffected |
13221441
| Process timeout | Kill process, return exit code 124 |

crates/openshell-sandbox/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ webpki-roots = { workspace = true }
5252
# HTTP
5353
bytes = { workspace = true }
5454

55+
# Encoding
56+
base64 = { workspace = true }
57+
5558
# IP network / CIDR parsing
5659
ipnet = "2"
5760

0 commit comments

Comments
 (0)