openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-08 20:40:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Harden credential handling in shell snapshots and replay (#44040)

Browse Source 
## Why

Shell quoting can hide credentials from raw-text checks, and startup files can restore real credentials after the broker replaces them with dummy values.

## What changed

- Decode shell literals without evaluating them and reject snapshots containing credentials in executable source, including aliases, functions, and heredocs.
- Preserve credential policy overrides, explicit unsets, and aliases whose source variables were removed. Support credential aliases in Zsh tied arrays while rejecting credentials that span array elements.
- Guard snapshot replay against credential restoration through shell startup files and preserve unrelated `ENV` settings.
- Apply Windows environment-key casing rules to credential overrides, suppress unredacted sandbox diagnostics during snapshot capture, and clear inherited environment variables before launching escalated commands.

## Testing

Add regression coverage for shell quoting and escaped credentials, Zsh tied arrays, startup-file replay, readonly credentials, policy overrides, and sensitive capture timeout and cancellation handling.

GitOrigin-RevId: 58274c07c715423241d26ce6dd2c2b4230cf0f64
This commit is contained in:
Winston Howes
2026-09-09 04:00:10 +00:00
committed by copyberry
parent a548463b78
commit ec512d2347
21 changed files with 4215 additions and 256 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/shell-command/src/shell_snapshot_credentials.rs
+126 -36
View File
@@ -1,7 +1,9 @@
//! Apply credential and environment policy to captured exports, producing typed render values.
//! Native declarations are rendered only after applying the supplied credential policy.
//! The shared literal decoder still validates alias and function bodies without evaluating them.
use super::capture::CapturedSnapshot;
use super::capture::ExportValue;
use super::literals;
use super::render;
use super::render::Export;
use super::render::Value;
@@ -32,7 +34,7 @@ pub struct PreparedSnapshot {
/// Prepare a replay script from captured state without changing the captured data.
///
/// Credential policy is applied before rendering. A rejected capture produces no script,
/// when the assembled source still contains a recognized credential.
/// including when executable source reconstructs a credential through shell quoting.
pub fn prepare_snapshot_credentials(
captured: &CapturedSnapshot<'_>,
environment: SnapshotCredentialEnvironment<'_>,
@@ -136,7 +138,7 @@ pub fn prepare_snapshot_credentials(
let mut alias_values = HashMap::new();
let mut rejected_alias_keys = Vec::new();
let mut invalid_export = false;
let mut exports = captured
let exports = captured
.exports
.iter()
.filter_map(|export| {
@@ -148,14 +150,46 @@ pub fn prepare_snapshot_credentials(
|| brokered_keys
.iter()
.any(|credential_key| credential_key == key)
|| brokered_alias_keys
.iter()
.any(|credential_key| credential_key == key)
|| is_disallowed_credential_alias(key, &mut virtualize_text)
{
return None;
}
if configured.contains_key(key)
&& let ExportValue::TiedArray(array) = &export.value
{
let value = allowed.get(key)?;
let mut virtualized = value.clone();
if !virtualize_text(&mut virtualized) || virtualized != *value {
invalid_export = true;
return Some(Export::Captured(line));
}
if credential_aliases.remove(key).is_some() {
alias_values.insert(key.to_string(), value.clone());
}
// An explicit scalar override replaces captured elements, not the array binding.
return Some(Export::ArrayBinding {
key,
declaration: line[..array.span.start].strip_suffix('=')?,
suffix: &line[array.span.end..],
});
}
if is_disallowed_credential_alias(key, &mut virtualize_text) {
return None;
}
if brokered_alias_keys.iter().any(|alias_key| alias_key == key) {
// Source-less aliases keep their live scalar value, including explicit unsets.
return match &export.value {
ExportValue::TiedArray(array) => Some(Export::ArrayBinding {
key,
declaration: line[..array.span.start].strip_suffix('=')?,
suffix: &line[array.span.end..],
}),
ExportValue::Plain | ExportValue::UnparsedArray => None,
};
}
if let Some(credential_keys) = credential_aliases.remove(key) {
if !credential_alias_is_allowed(&credential_keys) {
rejected_alias_keys.push(key.to_string());
@@ -167,12 +201,74 @@ pub fn prepare_snapshot_credentials(
let (assignment, value) =
credential_alias_assignment(allowed.get(key)?, &credential_keys)?;
alias_values.insert(key.to_string(), value);
if line
.split_whitespace()
.skip(/*n*/ 1)
.take_while(|word| word.starts_with('-'))
.any(|flags| flags.contains('T'))
{
if let ExportValue::TiedArray(array) = &export.value {
let scalar = array.values.join(array.separator.as_slice());
let crosses_element = credential_keys
.iter()
.flat_map(|key| {
[
real_credential_value(key),
original.get(*key),
discovered.get(*key),
]
})
.flatten()
.filter(|credential| !credential.is_empty())
.any(|credential| {
scalar
.windows(credential.len())
.enumerate()
.any(|(start, bytes)| {
if bytes != credential.as_bytes() {
return false;
}
let mut offset = 0;
!array.values.iter().any(|value| {
let contains = offset <= start
&& start + credential.len() <= offset + value.len();
offset += value.len() + array.separator.len();
contains
})
})
});
if crosses_element {
invalid_export = true;
return Some(Export::Captured(line));
}
let Some((elements, normalized)) = array
.values
.iter()
.map(|bytes| {
let mut text = String::from_utf8(bytes.clone()).ok()?;
if !virtualize_text(&mut text) {
return None;
}
credential_alias_assignment(&text, &credential_keys)
})
.collect::<Option<Vec<_>>>()
.map(|values| values.into_iter().unzip::<_, _, Vec<_>, Vec<_>>())
else {
invalid_export = true;
return Some(Export::Captured(line));
};
// Never replay a known credential that crosses array-element boundaries.
if normalized
.iter()
.map(String::as_bytes)
.collect::<Vec<_>>()
.join(array.separator.as_slice())
!= allowed.get(key)?.as_bytes()
{
invalid_export = true;
return Some(Export::Captured(line));
}
return Some(Export::Array {
prefix: &line[..array.span.start],
elements,
suffix: &line[array.span.end..],
});
}
if matches!(export.value, ExportValue::UnparsedArray) {
invalid_export = true;
return Some(Export::Captured(line));
}
@@ -185,6 +281,7 @@ pub fn prepare_snapshot_credentials(
Some(Export::Captured(line))
})
.collect::<Vec<_>>();
let snapshot = render::render(captured.state, captured.aliases, &exports)?;
if invalid_export {
return None;
}
@@ -192,38 +289,31 @@ pub fn prepare_snapshot_credentials(
for (key, credential_keys) in credential_aliases {
if !is_disallowed_credential_alias(key, &mut virtualize_text)
&& credential_alias_is_allowed(&credential_keys)
&& let Some((assignment, value)) = allowed
&& let Some((_, value)) = allowed
.get(key)
.and_then(|value| credential_alias_assignment(value, &credential_keys))
{
exports.push(Export::Alias {
key,
value: assignment,
});
// Native capture can deliberately omit readonly exports; retain only their metadata.
alias_values.insert(key.to_string(), value);
} else {
rejected_alias_keys.push(key.to_string());
}
}
let mut snapshot = render::render(captured.state, captured.aliases, &exports)?;
let mut credential_values = original
.values()
.chain(restored.values())
.chain(configured.values())
.collect::<Vec<_>>();
credential_values.sort_unstable_by_key(|value| (std::cmp::Reverse(value.len()), *value));
credential_values.dedup();
for value in credential_values {
let mut replacement = value.clone();
if virtualize_text(&mut replacement) && replacement != *value {
snapshot = snapshot.replace(value, &replacement);
}
let mut virtualized = snapshot.clone();
if !virtualize_text(&mut virtualized) || virtualized != snapshot {
return None;
}
Some(PreparedSnapshot {
script: snapshot,
aliases: alias_values,
rejected_alias_keys,
})
literals::literal_words(&snapshot, captured.shell_type)?
.into_iter()
.all(|word| {
let mut virtualized = word.clone();
virtualize_text(&mut virtualized) && virtualized == word
})
.then_some(PreparedSnapshot {
script: snapshot,
aliases: alias_values,
rejected_alias_keys,
})
}