Skip to content

have world_writable_warning_details accept cwd as a param #6913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2025
Merged

have world_writable_warning_details accept cwd as a param #6913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion codex-rs/app-server/src/message_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,15 @@ impl MessageProcessor {
}

// This function is stubbed out to return None on non-Windows platforms
let cwd = match std::env::current_dir() {
Copy link
Contributor Author

@iceweasel-oai iceweasel-oai Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dylan-hurd-oai this is where you'll want to update it to use the conversation's cwd

Copy link
Collaborator

@dylan-hurd-oai dylan-hurd-oai Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just pass in self.config.cwd here

Ok(cwd) => cwd,
Err(_) => return,
};
Comment on lines +194 to +197
Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve failed-scan notification

If std::env::current_dir() fails we now return early, so the client never receives the WindowsWorldWritableWarning notification. Before this change, world_writable_warning_details returned Some((Vec::new(), 0, true)), allowing the UI to display the failed-scan warning. Please keep propagating the failure (for example, by sending the notification with failed_scan = true) so users are still informed when the scan cannot run.

Useful? React with 👍 / 👎.

if let Some((sample_paths, extra_count, failed_scan)) =
codex_windows_sandbox::world_writable_warning_details(self.config.codex_home.as_path())
codex_windows_sandbox::world_writable_warning_details(
self.config.codex_home.as_path(),
cwd,
)
{
self.outgoing
.send_server_notification(ServerNotification::WindowsWorldWritableWarning(
Expand Down
6 changes: 5 additions & 1 deletion codex-rs/tui/src/chatwidget.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2300,7 +2300,11 @@ impl ChatWidget {
{
return None;
}
codex_windows_sandbox::world_writable_warning_details(self.config.codex_home.as_path())
let cwd = match std::env::current_dir() {
Copy link
Collaborator

@dylan-hurd-oai dylan-hurd-oai Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use config.cwd here

Ok(cwd) => cwd,
Err(_) => return Some((Vec::new(), 0, true)),
};
codex_windows_sandbox::world_writable_warning_details(self.config.codex_home.as_path(), cwd)
}

#[cfg(not(target_os = "windows"))]
Expand Down
70 changes: 34 additions & 36 deletions codex-rs/windows-sandbox-rs/src/audit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,35 @@ use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use std::time::Instant;
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Foundation::ERROR_SUCCESS;
use windows_sys::Win32::Foundation::HLOCAL;
use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
use windows_sys::Win32::Security::Authorization::GetNamedSecurityInfoW;
use windows_sys::Win32::Security::Authorization::GetSecurityInfo;
use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Storage::FileSystem::CreateFileW;
use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE;
use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA;
use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES;
use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
const GENERIC_ALL_MASK: u32 = 0x1000_0000;
const GENERIC_WRITE_MASK: u32 = 0x4000_0000;
use windows_sys::Win32::Security::ACL;
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
use windows_sys::Win32::Security::AclSizeInformation;
use windows_sys::Win32::Security::GetAclInformation;
use windows_sys::Win32::Security::EqualSid;
use windows_sys::Win32::Security::GetAce;
use windows_sys::Win32::Security::GetAclInformation;
use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE;
use windows_sys::Win32::Security::ACE_HEADER;
use windows_sys::Win32::Security::EqualSid;
use windows_sys::Win32::Security::ACL;
use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;

// Preflight scan limits
const MAX_ITEMS_PER_DIR: i32 = 1000;
Expand Down Expand Up @@ -162,7 +162,9 @@ unsafe fn path_has_world_write_allow(path: &Path) -> Result<bool> {
let psid_world = world.as_mut_ptr() as *mut c_void;
// Very fast mask-based check for world-writable grants (includes GENERIC_*).
if !dacl_quick_world_write_mask_allows(p_dacl, psid_world) {
if !p_sd.is_null() { LocalFree(p_sd as HLOCAL); }
if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL);
}
return Ok(false);
}
// Quick detector flagged a write grant for Everyone: treat as writable.
Expand Down Expand Up @@ -202,7 +204,9 @@ pub fn audit_everyone_writable(
let has = unsafe { path_has_world_write_allow(&p)? };
if has {
let key = normalize_path_key(&p);
if seen.insert(key) { flagged.push(p); }
if seen.insert(key) {
flagged.push(p);
}
}
}
}
Expand All @@ -218,7 +222,9 @@ pub fn audit_everyone_writable(
let has_root = unsafe { path_has_world_write_allow(&root)? };
if has_root {
let key = normalize_path_key(&root);
if seen.insert(key) { flagged.push(root.clone()); }
if seen.insert(key) {
flagged.push(root.clone());
}
}
// one level down best-effort
if let Ok(read) = std::fs::read_dir(&root) {
Expand All @@ -240,13 +246,17 @@ pub fn audit_everyone_writable(
// Skip noisy/irrelevant Windows system subdirectories
let pl = p.to_string_lossy().to_ascii_lowercase();
let norm = pl.replace('\\', "/");
if SKIP_DIR_SUFFIXES.iter().any(|s| norm.ends_with(s)) { continue; }
if SKIP_DIR_SUFFIXES.iter().any(|s| norm.ends_with(s)) {
continue;
}
if ft.is_dir() {
checked += 1;
let has_child = unsafe { path_has_world_write_allow(&p)? };
if has_child {
let key = normalize_path_key(&p);
if seen.insert(key) { flagged.push(p); }
if seen.insert(key) {
flagged.push(p);
}
}
}
}
Expand All @@ -258,20 +268,12 @@ pub fn audit_everyone_writable(
for p in &flagged {
list.push_str(&format!("\n - {}", p.display()));
}
crate::logging::log_note(
&format!(
"AUDIT: world-writable scan FAILED; checked={checked}; duration_ms={elapsed_ms}; flagged:{}",
list
),
logs_base_dir,
);

return Ok(flagged);
}
// Log success once if nothing flagged
crate::logging::log_note(
&format!(
"AUDIT: world-writable scan OK; checked={checked}; duration_ms={elapsed_ms}"
),
&format!("AUDIT: world-writable scan OK; checked={checked}; duration_ms={elapsed_ms}"),
Comment on lines 268 to +276
Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore failure logging

When flagged is non-empty we used to log the detailed failure via crate::logging::log_note, but that call disappeared in this revision. As a result we no longer emit the audit failure message to the logs, so operators lose visibility into which paths were world-writable. Please reinstate the logging before returning the flagged list.

Useful? React with 👍 / 👎.

logs_base_dir,
);
Ok(Vec::new())
Expand All @@ -284,14 +286,10 @@ fn normalize_windows_path_for_display(p: impl AsRef<Path>) -> String {

pub fn world_writable_warning_details(
codex_home: impl AsRef<Path>,
cwd: impl AsRef<Path>,
) -> Option<(Vec<String>, usize, bool)> {
let cwd = match std::env::current_dir() {
Ok(cwd) => cwd,
Err(_) => return Some((Vec::new(), 0, true)),
};

let env_map: HashMap<String, String> = std::env::vars().collect();
match audit_everyone_writable(&cwd, &env_map, Some(codex_home.as_ref())) {
match audit_everyone_writable(cwd.as_ref(), &env_map, Some(codex_home.as_ref())) {
Ok(paths) if paths.is_empty() => None,
Ok(paths) => {
let as_strings: Vec<String> = paths
Expand Down Expand Up @@ -329,16 +327,16 @@ unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut
continue;
}
let hdr = &*(p_ace as *const ACE_HEADER);
if hdr.AceType != 0 { // ACCESS_ALLOWED_ACE_TYPE
if hdr.AceType != 0 {
// ACCESS_ALLOWED_ACE_TYPE
continue;
}
if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 {
continue;
}
let base = p_ace as usize;
let sid_ptr = (base
+ std::mem::size_of::<ACE_HEADER>()
+ std::mem::size_of::<u32>()) as *mut c_void; // skip header + mask
let sid_ptr =
(base + std::mem::size_of::<ACE_HEADER>() + std::mem::size_of::<u32>()) as *mut c_void; // skip header + mask
if EqualSid(sid_ptr, psid_world) != 0 {
let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE);
let mask = ace.Mask;
Expand Down
1 change: 1 addition & 0 deletions codex-rs/windows-sandbox-rs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ mod stub {

pub fn world_writable_warning_details(
_codex_home: impl AsRef<Path>,
_cwd: impl AsRef<Path>,
) -> Option<(Vec<String>, usize, bool)> {
None
}
Expand Down
Loading