fix(sandbox): canonicalize HTTP request-targets before L7 policy evaluation (#878)

* fix(sandbox): canonicalize HTTP request-targets before L7 policy evaluation

The L7 REST proxy evaluated OPA path rules against the raw request-target
and forwarded the raw header block byte-for-byte to the upstream. The
upstream normalized `..`, `%2e%2e`, and `//` before dispatching, so a
compromised agent inside the sandbox could bypass any path-based allow
rule by prefixing the blocked path with an allowed one (e.g.
`GET /public/../secret` matches `/public/**` at the proxy but the
upstream serves `/secret`). The inference-routing detection had the same
normalization gap via `normalize_inference_path`, which only stripped
scheme+authority and preserved dot-segments verbatim.

- Add `crates/openshell-sandbox/src/l7/path.rs` with
  `canonicalize_request_target`. Percent-decodes (with uppercase hex
  re-emission), resolves `.` / `..` segments per RFC 3986 5.2.4,
  collapses doubled slashes, strips trailing `;params`, rejects
  fragments / raw non-ASCII / control bytes / encoded slashes (unless
  explicitly enabled per-endpoint), and returns a `rewritten` flag so
  callers know when to rewrite the outbound request line.
- Wire the canonicalizer into `rest.rs::parse_http_request`. The
  canonical path is the `target` on `L7Request` that OPA sees, and
  when the canonical form differs from the raw input the request line
  is rebuilt in `raw_header` so the upstream dispatches on the exact
  bytes the policy evaluated.
- Canonicalize the forward (plain HTTP proxy) path at
  `proxy.rs`'s second L7 evaluation site. A non-canonical request-target
  is rejected with 400 Bad Request and an OCSF `NetworkActivity` Fail
  event rather than silently passed through.
- Replace the old `normalize_inference_path` body with a call to the
  canonicalizer. On canonicalization error the raw path is returned
  (so inference detection simply misses and the normal forward path
  handles and rejects).
- Document the invariant in `sandbox-policy.rego`: `input.request.path`
  is pre-canonicalized so rules must not attempt defensive matching
  against `..` or `%2e%2e`.
- Architecture doc (`architecture/sandbox.md`) lists the new module.
- 24 new unit tests cover dot segments, percent-encoded dot segments,
  double-slash collapse, encoded-slash reject/opt-in, null/control byte
  rejection, legitimate percent-encoded bytes round-tripping, mixed-case
  percent normalization, fragment rejection, absolute-form stripping,
  empty / length-bounded / non-ASCII handling, and regression payloads
  for the specific bypasses above.

Tracks OS-99.

* fix(sandbox): close forward-proxy parser-differential and wire allow_encoded_slash

Addresses two review findings on the L7 path canonicalization change.

1. `handle_forward_proxy` previously evaluated OPA on the canonical path
   but wrote the raw path to the upstream via `rewrite_forward_request`,
   leaving the parser-differential open for plain-HTTP forward-proxy
   traffic even though it was closed for the L7 tunnel path. `path` is
   now reassigned to the canonical form inside the L7 block before the
   outbound write, so the bytes dispatched to the upstream match what
   OPA evaluated. Covered by a new regression test against
   `rewrite_forward_request`.

2. `allow_encoded_slash` is now wired through the YAML policy schema,
   the proto `NetworkEndpoint` message, the Rego endpoint config, and
   the `L7EndpointConfig` → `RestProvider` canonicalization options.
   Operators can opt in per-endpoint for upstreams like GitLab that
   embed `%2F` in namespaced resource paths; the default remains
   strict. The `RestProvider` gains a constructor
   `RestProvider::with_options` so `relay_rest` constructs a provider
   with per-endpoint options while `relay_passthrough_with_credentials`
   retains strict defaults.

Integration tests:
- `parse_http_request_canonicalizes_target_and_rewrites_raw_header`
  drives a non-canonical request through the parser and asserts both
  `L7Request.target` (what OPA evaluates) and `raw_header` (what the
  upstream receives) are canonical.
- `parse_http_request_canonicalization_preserves_query_string`,
  `parse_http_request_leaves_canonical_input_byte_for_byte`,
  `parse_http_request_rejects_traversal_above_root`,
  `parse_http_request_preserves_http_10_version_on_rewrite`.
- `parse_http_request_accepts_encoded_slash_when_endpoint_opts_in` /
  `_rejects_encoded_slash_by_default` verify the
  `allow_encoded_slash` gate.
- `test_rewrite_forward_request_uses_canonical_path_on_the_wire`
  asserts the fix to the forward-proxy flow.
- `parse_l7_config_allow_encoded_slash_{defaults_false,opt_in}` verify
  the YAML/Rego wiring.

Author: johntmyers

architecture/sandbox.md

@@ -32,6 +32,7 @@ All paths are relative to `crates/openshell-sandbox/src/`.
 | `l7/tls.rs` | Ephemeral CA generation (`SandboxCa`), per-hostname leaf cert cache (`CertCache`), TLS termination/connection helpers, `looks_like_tls()` auto-detection |
 | `l7/relay.rs` | Protocol-aware bidirectional relay with per-request OPA evaluation, credential-injection-only passthrough relay |
 | `l7/rest.rs` | HTTP/1.1 request/response parsing, body framing (Content-Length, chunked), deny response generation |
+| `l7/path.rs` | Request-target canonicalization: percent-decoding, dot-segment resolution, `;params` stripping, encoded-slash policy (opt-in per endpoint via `allow_encoded_slash: true` for upstreams like GitLab that embed `%2F` in paths). Single source of truth for the path both OPA evaluates and the upstream receives. |
 | `l7/provider.rs` | `L7Provider` trait and `L7Request`/`BodyLength` types |
 | `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 |
 

crates/openshell-policy/src/lib.rs

@@ -109,6 +109,12 @@ struct NetworkEndpointDef {
     allowed_ips: Vec<String>,
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     deny_rules: Vec<L7DenyRuleDef>,
+    /// When true, percent-encoded `/` (`%2F`) is preserved in path segments
+    /// rather than rejected by the L7 path canonicalizer. Required for
+    /// upstreams like GitLab that embed `%2F` in namespaced resource paths.
+    /// Defaults to false (strict).
+    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+    allow_encoded_slash: bool,
 }
 
 fn is_zero(v: &u16) -> bool {
@@ -261,6 +267,7 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy {
                                         .collect(),
                                 })
                                 .collect(),
+                            allow_encoded_slash: e.allow_encoded_slash,
                         }
                     })
                     .collect(),
@@ -400,6 +407,7 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
                                         .collect(),
                                 })
                                 .collect(),
+                            allow_encoded_slash: e.allow_encoded_slash,
                         }
                     })
                     .collect(),

crates/openshell-sandbox/data/sandbox-policy.rego

@@ -328,6 +328,13 @@ method_matches(actual, expected) if {
 }
 
 # Path matching: "**" matches everything; otherwise glob.match with "/" delimiter.
+#
+# INVARIANT: `input.request.path` is canonicalized by the sandbox before
+# policy evaluation — percent-decoded, dot-segments resolved, doubled
+# slashes collapsed, `;params` stripped, `%2F` rejected (unless an
+# endpoint opts in). Patterns here must therefore match canonical paths;
+# do not attempt defensive matching against `..` or `%2e%2e` — those
+# inputs are rejected at the L7 parser boundary before this rule runs.
 path_matches(_, "**") if true
 
 path_matches(actual, pattern) if {
@@ -394,8 +401,8 @@ command_matches(actual, expected) if {
 
 # --- Matched endpoint config (for L7 and allowed_ips extraction) ---
 # Returns the raw endpoint object for the matched policy + host:port.
-# Used by Rust to extract L7 config (protocol, tls, enforcement) and/or
-# allowed_ips for SSRF allowlist validation.
+# Used by Rust to extract L7 config (protocol, tls, enforcement,
+# allow_encoded_slash) and/or allowed_ips for SSRF allowlist validation.
 
 # Per-policy helper: returns matching endpoint configs for a single policy.
 _policy_endpoint_configs(policy) := [ep |

crates/openshell-sandbox/src/l7/mod.rs

@@ -9,6 +9,7 @@
 //! evaluated against OPA policy, and either forwarded or denied.
 
 pub mod inference;
+pub mod path;
 pub mod provider;
 pub mod relay;
 pub mod rest;
@@ -59,6 +60,10 @@ pub struct L7EndpointConfig {
     pub protocol: L7Protocol,
     pub tls: TlsMode,
     pub enforcement: EnforcementMode,
+    /// When true, percent-encoded `/` (`%2F`) is preserved in path segments
+    /// rather than rejected at the parser. Needed by upstreams like GitLab
+    /// that embed `%2F` in namespaced project paths. Defaults to false.
+    pub allow_encoded_slash: bool,
 }
 
 /// Result of an L7 policy decision for a single request.
@@ -122,10 +127,13 @@ pub fn parse_l7_config(val: &regorus::Value) -> Option<L7EndpointConfig> {
         _ => EnforcementMode::Audit,
     };
 
+    let allow_encoded_slash = get_object_bool(val, "allow_encoded_slash").unwrap_or(false);
+
     Some(L7EndpointConfig {
         protocol,
         tls,
         enforcement,
+        allow_encoded_slash,
     })
 }
 
@@ -141,6 +149,19 @@ pub fn parse_tls_mode(val: &regorus::Value) -> TlsMode {
     }
 }
 
+/// Extract a bool value from a regorus object. Returns `None` when the key
+/// is absent or not a boolean.
+fn get_object_bool(val: &regorus::Value, key: &str) -> Option<bool> {
+    let key_val = regorus::Value::String(key.into());
+    match val {
+        regorus::Value::Object(map) => match map.get(&key_val) {
+            Some(regorus::Value::Bool(b)) => Some(*b),
+            _ => None,
+        },
+        _ => None,
+    }
+}
+
 /// Extract a string value from a regorus object.
 fn get_object_str(val: &regorus::Value, key: &str) -> Option<String> {
     let key_val = regorus::Value::String(key.into());
@@ -746,6 +767,26 @@ mod tests {
         assert!(parse_l7_config(&val).is_none());
     }
 
+    #[test]
+    fn parse_l7_config_allow_encoded_slash_defaults_false() {
+        let val = regorus::Value::from_json_str(
+            r#"{"protocol": "rest", "host": "api.example.com", "port": 443}"#,
+        )
+        .unwrap();
+        let config = parse_l7_config(&val).unwrap();
+        assert!(!config.allow_encoded_slash);
+    }
+
+    #[test]
+    fn parse_l7_config_allow_encoded_slash_opt_in() {
+        let val = regorus::Value::from_json_str(
+            r#"{"protocol": "rest", "host": "gitlab.example.com", "port": 443, "allow_encoded_slash": true}"#,
+        )
+        .unwrap();
+        let config = parse_l7_config(&val).unwrap();
+        assert!(config.allow_encoded_slash);
+    }
+
     #[test]
     fn validate_rules_and_access_mutual_exclusion() {
         let data = serde_json::json!({