mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Handle copied credentials in the broker and shell snapshots (#44038)
## Why Shell startup can copy a credential into another variable and unset its original source. These aliases need credential brokering without losing source restrictions or mistaking ordinary shell content for credentials. ## What changed - Discover supported GitHub and OpenAI credentials embedded in environment values, even without their canonical variables. Preserve source ownership so unbound enterprise tokens cannot acquire a default host binding. - Redact unregistered supported credentials during text virtualization, distinguish adjacent credentials, and avoid matching unrelated provider prefixes and common hashed paths. - Expose helpers for checking allowed credential sources and restoring known dummy credentials in trusted text for fail-open execution. - Render allowed credential aliases as snapshot exports and replace known credential values in shell state, preserving exported functions and credential-shaped function names. ## Testing Add regression coverage for copied aliases, enterprise host binding, source filtering, adjacent tokens, path false positives, and dummy restoration. Add a Bash snapshot test that preserves exported functions while replacing credentials in heredoc content. GitOrigin-RevId: f1c3d531405a96ac6bfa3882a11b74643fa6f123
This commit is contained in:
@@ -136,7 +136,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 exports = captured
|
||||
let mut exports = captured
|
||||
.exports
|
||||
.iter()
|
||||
.filter_map(|export| {
|
||||
@@ -185,7 +185,6 @@ pub fn prepare_snapshot_credentials(
|
||||
Some(Export::Captured(line))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let mut snapshot = render::render(captured.state, captured.aliases, &exports)?;
|
||||
if invalid_export {
|
||||
return None;
|
||||
}
|
||||
@@ -193,19 +192,33 @@ 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((_, value)) = allowed
|
||||
&& let Some((assignment, value)) = allowed
|
||||
.get(key)
|
||||
.and_then(|value| credential_alias_assignment(value, &credential_keys))
|
||||
{
|
||||
// Native capture can deliberately omit readonly exports; retain only their metadata.
|
||||
exports.push(Export::Alias {
|
||||
key,
|
||||
value: assignment,
|
||||
});
|
||||
alias_values.insert(key.to_string(), value);
|
||||
} else {
|
||||
rejected_alias_keys.push(key.to_string());
|
||||
}
|
||||
}
|
||||
|
||||
if !virtualize_text(&mut snapshot) {
|
||||
return None;
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
Some(PreparedSnapshot {
|
||||
|
||||
@@ -32,6 +32,7 @@ pub(super) enum ValuePart {
|
||||
|
||||
pub(super) enum Export<'a> {
|
||||
Captured(&'a str),
|
||||
Alias { key: &'a str, value: Value },
|
||||
Assignment { declaration: &'a str, value: Value },
|
||||
}
|
||||
|
||||
@@ -63,6 +64,9 @@ pub(super) fn render(state: &str, aliases: &str, exports: &[Export<'_>]) -> Opti
|
||||
for export in exports {
|
||||
match export {
|
||||
Export::Captured(source) => output.push_str(source),
|
||||
Export::Alias { key, value } => {
|
||||
output.push_str(&format!("export {key}={}\n", value.render()?));
|
||||
}
|
||||
Export::Assignment { declaration, value } => {
|
||||
output.push_str(&format!("{declaration}={}\n", value.render()?));
|
||||
}
|
||||
|
||||
@@ -698,3 +698,85 @@ fn captured_script(shell_type: ShellType, source: &str) -> Result<String> {
|
||||
CapturedSnapshot::parse(shell_type, source.as_bytes()).context("invalid native capture")?;
|
||||
Ok(captured.render_script())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn brokered_bash_snapshot_preserves_exported_functions_and_redacts_heredocs() -> Result<()> {
|
||||
let dir = tempdir()?;
|
||||
let startup = dir.path().join("startup.sh");
|
||||
let real = "ghp_abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGH";
|
||||
let dummy = "ghp_brokered_dummy_abcdefghijklmnopqrstuvwxyz0123";
|
||||
let credential_shaped_function = "ghp_0123456789abcdefghijklmnopqrstuvwxyzABCD";
|
||||
std::fs::write(
|
||||
&startup,
|
||||
format!(
|
||||
"helper() {{\n cat <<'EOF'\nexport LEGACY_SETTING=production\nAuthorization: {real}\nEOF\n}}\n{credential_shaped_function}() {{ printf UNEXPECTED_EXECUTION; }}\nexport -f helper\nexport GH_TOKEN='{real}'\n"
|
||||
),
|
||||
)?;
|
||||
let output = Command::new("/bin/bash")
|
||||
.arg("-c")
|
||||
.arg(
|
||||
snapshot_capture_script(
|
||||
ShellType::Bash,
|
||||
SnapshotCaptureOptions {
|
||||
startup: SnapshotStartup::NonInteractive,
|
||||
..CAPTURE_ALL
|
||||
},
|
||||
)
|
||||
.expect("bash supports snapshots"),
|
||||
)
|
||||
.env_clear()
|
||||
.env("HOME", dir.path())
|
||||
.env("PATH", "/usr/bin:/bin")
|
||||
.env("BASH_ENV", &startup)
|
||||
.output()?;
|
||||
assert!(output.status.success());
|
||||
|
||||
let mut snapshot = String::from_utf8(output.stdout)?;
|
||||
let original = HashMap::from([("GH_TOKEN".to_string(), real.to_string())]);
|
||||
let discovered = HashMap::from([("GH_TOKEN".to_string(), dummy.to_string())]);
|
||||
let brokered_keys = vec!["GH_TOKEN".to_string()];
|
||||
rewrite_snapshot_credentials(
|
||||
ShellType::Bash,
|
||||
&mut snapshot,
|
||||
SnapshotCredentialEnvironment {
|
||||
original: &original,
|
||||
restored: &original,
|
||||
configured: &HashMap::new(),
|
||||
discovered: &discovered,
|
||||
allowed: &discovered,
|
||||
is_allowed_unset: &|_| false,
|
||||
brokered_keys: &brokered_keys,
|
||||
brokered_alias_keys: &[],
|
||||
allowed_brokered_keys: &brokered_keys,
|
||||
},
|
||||
|text| {
|
||||
if text.contains(credential_shaped_function) {
|
||||
*text = text.replace(credential_shaped_function, "");
|
||||
return false;
|
||||
}
|
||||
*text = text.replace(real, dummy);
|
||||
true
|
||||
},
|
||||
);
|
||||
assert!(!snapshot.contains(real));
|
||||
assert!(snapshot.contains(credential_shaped_function));
|
||||
let path = dir.path().join("snapshot.sh");
|
||||
std::fs::write(&path, snapshot)?;
|
||||
|
||||
let restored = Command::new("/bin/bash")
|
||||
.arg("-c")
|
||||
.arg(". \"$1\"; exec /bin/bash -c helper")
|
||||
.arg("snapshot")
|
||||
.arg(&path)
|
||||
.env_clear()
|
||||
.env("PATH", "/usr/bin:/bin")
|
||||
.env("GH_TOKEN", dummy)
|
||||
.output()?;
|
||||
assert!(restored.status.success());
|
||||
assert_eq!(
|
||||
String::from_utf8(restored.stdout)?,
|
||||
format!("export LEGACY_SETTING=production\nAuthorization: {dummy}\n")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user