Skip to content

Commit 2ab0da7

Browse files
committed
fix(providersv2): make inject_header byte-oriented and fix import diagnostic wording
Signed-off-by: Calum Murray <cmurray@redhat.com>
1 parent 6360900 commit 2ab0da7

2 files changed

Lines changed: 71 additions & 19 deletions

File tree

crates/openshell-server/src/grpc/provider.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,9 +1738,9 @@ async fn profile_import_attached_sandbox_diagnostics(
17381738
diagnostics.push(ProfileValidationDiagnostic {
17391739
source: source.clone(),
17401740
profile_id: profile_id.clone(),
1741-
field: "credentials.token_grant.audience_overrides".to_string(),
1741+
field: "credentials".to_string(),
17421742
message: format!(
1743-
"import would create ambiguous dynamic token grants on sandbox '{sandbox_name}': {}",
1743+
"import would create ambiguous provider credential bindings on sandbox '{sandbox_name}': {}",
17441744
err.message()
17451745
),
17461746
severity: "error".to_string(),
@@ -3042,7 +3042,7 @@ mod tests {
30423042
assert!(response.diagnostics.iter().any(|diagnostic| {
30433043
diagnostic
30443044
.message
3045-
.contains("import would create ambiguous dynamic token grants")
3045+
.contains("import would create ambiguous provider credential bindings")
30463046
}));
30473047
}
30483048

crates/openshell-supervisor-network/src/l7/token_grant_injection.rs

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -532,39 +532,46 @@ fn validate_header_name(header_name: &str) -> Result<()> {
532532
}
533533
}
534534

535+
/// Injects (or replaces) a header in the raw HTTP request.
536+
///
537+
/// Operates byte-wise so that non-UTF-8 obs-text bytes in unrelated
538+
/// headers do not cause the injection to fail. Only the header name
539+
/// comparison is ASCII — header values are preserved as raw bytes.
535540
fn inject_header(raw_header: &[u8], header_name: &str, header_value: &str) -> Result<Vec<u8>> {
536541
let header_end = raw_header
537542
.windows(4)
538543
.position(|w| w == b"\r\n\r\n")
539544
.ok_or_else(|| miette!("HTTP headers missing final CRLF CRLF"))?;
540545

541-
let header_block = std::str::from_utf8(&raw_header[..header_end])
542-
.map_err(|_| miette!("HTTP headers contain invalid UTF-8"))?;
543-
let mut lines = header_block.split("\r\n");
544-
let request_line = lines
545-
.next()
546-
.ok_or_else(|| miette!("HTTP headers missing request line"))?;
547-
546+
let header_name_bytes = header_name.as_bytes();
548547
let inserted_header = format!("{header_name}: {header_value}");
549548
let mut new_raw_header = Vec::with_capacity(raw_header.len() + inserted_header.len() + 2);
550-
new_raw_header.extend_from_slice(request_line.as_bytes());
551-
new_raw_header.extend_from_slice(b"\r\n");
552549

553-
for line in lines {
550+
let mut first_line = true;
551+
for line in raw_header[..header_end].split(|&b| b == b'\n') {
552+
let line = line.strip_suffix(b"\r").unwrap_or(line);
553+
if first_line {
554+
// Request line — always preserve.
555+
first_line = false;
556+
new_raw_header.extend_from_slice(line);
557+
new_raw_header.extend_from_slice(b"\r\n");
558+
continue;
559+
}
554560
if line.is_empty() {
555561
break;
556562
}
557-
if line
558-
.split_once(':')
559-
.is_some_and(|(name, _)| name.trim().eq_ignore_ascii_case(header_name))
560-
{
561-
continue;
563+
if let Some(colon_pos) = line.iter().position(|&b| b == b':') {
564+
let name = trim_ascii(&line[..colon_pos]);
565+
if name.eq_ignore_ascii_case(header_name_bytes) {
566+
continue; // drop the existing header — we'll append the new one
567+
}
562568
}
563-
new_raw_header.extend_from_slice(line.as_bytes());
569+
new_raw_header.extend_from_slice(line);
564570
new_raw_header.extend_from_slice(b"\r\n");
565571
}
566572

567573
new_raw_header.extend_from_slice(inserted_header.as_bytes());
574+
// Preserve the original terminator and any trailing body bytes.
568575
new_raw_header.extend_from_slice(&raw_header[header_end..]);
569576

570577
Ok(new_raw_header)
@@ -1338,6 +1345,51 @@ mod tests {
13381345
);
13391346
}
13401347

1348+
#[tokio::test]
1349+
async fn inject_static_credential_succeeds_with_non_utf8_obs_text_in_unrelated_header() {
1350+
// A request with non-UTF-8 obs-text in an unrelated header must NOT
1351+
// cause inject_header to fail. The injection should succeed and
1352+
// preserve the non-UTF-8 bytes in the unrelated header.
1353+
let (ctx, _) =
1354+
static_credential_ctx("GITHUB_TOKEN", "ghp_secret123", "bearer", "Authorization");
1355+
let mut raw = b"GET /repos/owner/repo HTTP/1.1\r\nHost: api.example.com\r\n".to_vec();
1356+
// obs-text byte (0x80) in a different header — valid HTTP but not valid UTF-8.
1357+
raw.extend_from_slice(b"X-Custom: value\x80rest\r\n");
1358+
raw.extend_from_slice(b"\r\n");
1359+
let req = L7Request {
1360+
action: "GET".to_string(),
1361+
target: "/repos/owner/repo".to_string(),
1362+
query_params: std::collections::HashMap::new(),
1363+
raw_header: raw,
1364+
body_length: BodyLength::None,
1365+
};
1366+
1367+
let result = inject_if_needed(req, &ctx)
1368+
.await
1369+
.expect("injection must succeed despite non-UTF-8 in unrelated header");
1370+
1371+
// The injected header must be present.
1372+
let header_end = result
1373+
.raw_header
1374+
.windows(4)
1375+
.position(|w| w == b"\r\n\r\n")
1376+
.expect("must have header terminator");
1377+
let header_block = &result.raw_header[..header_end];
1378+
assert!(
1379+
header_block
1380+
.windows(b"Authorization: Bearer ghp_secret123".len())
1381+
.any(|w| w == b"Authorization: Bearer ghp_secret123"),
1382+
"injected Authorization header must be present"
1383+
);
1384+
// The non-UTF-8 header must be preserved as raw bytes.
1385+
assert!(
1386+
header_block
1387+
.windows(b"X-Custom: value\x80rest".len())
1388+
.any(|w| w == b"X-Custom: value\x80rest"),
1389+
"non-UTF-8 obs-text header must be preserved"
1390+
);
1391+
}
1392+
13411393
#[tokio::test]
13421394
async fn inject_if_needed_passthrough_preserves_graphql_body_in_raw_header() {
13431395
// GraphQL inspection appends the full request body to raw_header.

0 commit comments

Comments
 (0)