-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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.
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
Conversation
| } | ||
|
|
||
| // This function is stubbed out to return None on non-Windows platforms | ||
| let cwd = match std::env::current_dir() { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
| return None; | ||
| } | ||
| codex_windows_sandbox::world_writable_warning_details(self.config.codex_home.as_path()) | ||
| let cwd = match std::env::current_dir() { |
There was a problem hiding this comment.
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
| } | ||
|
|
||
| // This function is stubbed out to return None on non-Windows platforms | ||
| let cwd = match std::env::current_dir() { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let cwd = match std::env::current_dir() { | ||
| Ok(cwd) => cwd, | ||
| Err(_) => return, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 👍 / 👎.
| 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}"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 👍 / 👎.
this enables app-server to pass in the correct workspace cwd for the current conversation