From 528f9d50deabf1606e128d68e299f4db65cdaeec Mon Sep 17 00:00:00 2001 From: XSpielinbox <55600187+xspielinbox@users.noreply.github.com> Date: Wed, 13 May 2026 21:59:26 +0200 Subject: [PATCH 1/5] Create separate permission for extern usage --- src/checker.rs | 6 ++ src/config.rs | 3 + src/config/permissions.rs | 7 ++ src/config_editor.rs | 28 +++++++ src/extern_checker.rs | 136 ++++++++++++++++++++++++++++++++ src/main.rs | 1 + src/problem.rs | 16 +++- src/proxy/errors.rs | 38 ++++++++- src/proxy/rpc.rs | 27 ++++++- src/proxy/subprocess.rs | 39 ++++++++- src/summary.rs | 3 + src/ui/full_term/problems_ui.rs | 45 +++++++++-- src/unsafe_checker.rs | 124 +++++++++++++++++++++++++---- 13 files changed, 447 insertions(+), 26 deletions(-) create mode 100644 src/extern_checker.rs diff --git a/src/checker.rs b/src/checker.rs index a08ccd9..a166f44 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -20,6 +20,7 @@ use crate::problem::ProblemList; use crate::problem::UnusedAllowApi; use crate::proxy::cargo::profile_name; use crate::proxy::rpc; +use crate::proxy::rpc::ExternUsage; use crate::proxy::rpc::UnsafeUsage; use crate::proxy::subprocess::SubprocessConfig; use crate::symbol_graph::NameSource; @@ -238,6 +239,7 @@ impl Checker { }; match request { rpc::Request::CrateUsesUnsafe(usage) => Ok(self.crate_uses_unsafe(usage)), + rpc::Request::CrateUsesExtern(usage) => Ok(self.crate_uses_extern(usage)), rpc::Request::LinkerInvoked(link_info) => { self.outstanding_linker_invocations.push(link_info.clone()); Ok(ProblemList::default()) @@ -343,6 +345,10 @@ impl Checker { Problem::DisallowedUnsafe(usage.clone()).into() } + pub(crate) fn crate_uses_extern(&self, usage: &ExternUsage) -> ProblemList { + Problem::DisallowedExtern(usage.clone()).into() + } + pub(crate) fn verify_build_script_permitted(&mut self, pkg_id: &PackageId) -> ProblemList { if !self.config.raw.common.explicit_build_scripts { return ProblemList::default(); diff --git a/src/config.rs b/src/config.rs index cd7280e..d4952bf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -146,6 +146,9 @@ pub(crate) struct PackageConfig { #[serde(default)] pub(crate) allow_unsafe: bool, + #[serde(default)] + pub(crate) allow_extern: bool, + #[serde(default)] pub(crate) allow_build_instructions: Vec, diff --git a/src/config/permissions.rs b/src/config/permissions.rs index cebac77..0797dde 100644 --- a/src/config/permissions.rs +++ b/src/config/permissions.rs @@ -121,6 +121,12 @@ impl Permissions { .is_some_and(|crate_config| crate_config.allow_unsafe) } + pub(crate) fn extern_permitted_for_crate(&self, crate_sel: &CrateSel) -> bool { + self.packages + .get(&PermSel::for_non_build_output(crate_sel)) + .is_some_and(|crate_config| crate_config.allow_extern) + } + pub(crate) fn get(&self, perm_sel: &PermSel) -> Option<&PackageConfig> { self.packages.get(perm_sel) } @@ -184,6 +190,7 @@ impl PackageConfig { ); self.allow_proc_macro |= other.allow_proc_macro; self.allow_unsafe |= other.allow_unsafe; + self.allow_extern |= other.allow_extern; self.sandbox.inherit(&other.sandbox); } } diff --git a/src/config_editor.rs b/src/config_editor.rs index 96a9c42..d46e75b 100644 --- a/src/config_editor.rs +++ b/src/config_editor.rs @@ -111,6 +111,9 @@ pub(crate) fn fixes_for_problem(problem: &Problem, config: &Config) -> Vec edits.push(Box::new(AllowUnsafe { perm_sel: PermSel::for_non_build_output(&failure.crate_sel), })), + Problem::DisallowedExtern(failure) => edits.push(Box::new(AllowExtern { + perm_sel: PermSel::for_non_build_output(&failure.crate_sel), + })), Problem::UnusedAllowApi(failure) => edits.push(Box::new(RemoveUnusedAllowApis { unused: failure.clone(), })), @@ -1086,6 +1089,10 @@ struct AllowUnsafe { perm_sel: PermSel, } +struct AllowExtern { + perm_sel: PermSel, +} + impl Edit for AllowUnsafe { fn title(&self) -> String { format!("Allow package `{}` to use unsafe code", self.perm_sel) @@ -1107,6 +1114,27 @@ impl Edit for AllowUnsafe { } } +impl Edit for AllowExtern { + fn title(&self) -> String { + format!("Allow package `{}` to use extern code", self.perm_sel) + } + + fn help(&self) -> Cow<'static, str> { + "Allow this crate to use extern code. With extern code, this crate could do just about \ + anything, so this is like a bit like a wildcard permission. Crates that use extern \ + sometimes export APIs that you might want to restrict - e.g. network or filesystem APIs. \ + so you should have a think about if this crate falls into that category and if it does, \ + add some API definitions for it." + .into() + } + + fn apply(&self, editor: &mut ConfigEditor, opts: &EditOpts) -> Result<()> { + let table = editor.pkg_table(&self.perm_sel)?; + set_table_value(table, "allow_extern", toml_edit::value(true), opts); + Ok(()) + } +} + struct SandboxAllowNetwork { perm_sel: PermSel, } diff --git a/src/extern_checker.rs b/src/extern_checker.rs new file mode 100644 index 0000000..d5c1868 --- /dev/null +++ b/src/extern_checker.rs @@ -0,0 +1,136 @@ +//! This module tokenises Rust code and looks for the extern blocks. This is done as an additional +//! layer of defence in addition to use of the -Fmissing-unsafe-on-extern flag when compiling crates, +//! since that flag only works on code before Rust edition 2024. + +use crate::location::SourceLocation; +use anyhow::Context; +use anyhow::Result; +use ra_ap_rustc_lexer::Token; +use ra_ap_rustc_lexer::TokenKind; +use std::path::Path; + +/// Returns the locations of all extern block usages found in `path` +pub(crate) fn scan_path(path: &Path) -> Result> { + let bytes = + std::fs::read(path).with_context(|| format!("Failed to read `{}`", path.display()))?; + let Ok(source) = std::str::from_utf8(&bytes) else { + // If the file isn't valid UTF-8 then we don't need to check it for the extern blocks, + // since it can't be a source file that the rust compiler would accept. + return Ok(Vec::new()); + }; + Ok(scan_string(source, path)) +} + +fn scan_string(source: &str, path: &Path) -> Vec { + let mut token_start_offset = 0; + let mut locations = Vec::new(); + + let skip_condition = |x| { + matches!( + x, + TokenKind::BlockComment { + doc_style: _, + terminated: _ + } | TokenKind::LineComment { doc_style: _ } + | TokenKind::Whitespace + ) + }; + + let mut iter = ra_ap_rustc_lexer::tokenize(source, ra_ap_rustc_lexer::FrontmatterAllowed::No); + let mut previous_token_text = ""; + let mut previous_token_end_offset = 0; + let mut previous_token_length = 0; + + while let (Some(token), skipped_offset) = next_token(&mut iter, skip_condition) { + let token_length = usize::try_from(token.len).unwrap(); + + token_start_offset += skipped_offset; + + let token_end_offset = token_start_offset + token_length; + let token_text = &source[token_start_offset..token_end_offset]; + if !previous_token_text.is_empty() { + // check against previous token + if check_tokens(previous_token_text, token_text) { + add_location( + source, + path, + &mut locations, + previous_token_end_offset, + previous_token_length, + ); + } + } + previous_token_text = token_text; + previous_token_end_offset = token_end_offset; + previous_token_length = token_length; + // always check against potential future token + if let (Some(next_token), skipped_offset) = next_token(&mut iter, skip_condition) { + // the next token starts after the end of the current token + token_start_offset += token_length; + token_start_offset += skipped_offset; + + let next_token_length = usize::try_from(next_token.len).unwrap(); + + let next_token_end_offset = token_start_offset + next_token_length; + let next_token_text = &source[token_start_offset..next_token_end_offset]; + if check_tokens(token_text, next_token_text) { + add_location(source, path, &mut locations, token_end_offset, token_length); + } + token_start_offset = next_token_end_offset; + // as we consumed the next token already, this will be our previous token in the next loop iteration + previous_token_text = next_token_text; + previous_token_end_offset = next_token_end_offset; + previous_token_length = next_token_length; + } else { + // current token is last token in file + // this should never be valid code, but we flag it nevertheless to be safe + if token_text == "extern" { + add_location(source, path, &mut locations, token_end_offset, token_length); + } + // as there should not be another loop iteration this should be useless + // we do it just for completeness + token_start_offset = token_end_offset; + } + } + locations +} + +fn check_tokens(first_token_text: &str, second_token_text: &str) -> bool { + first_token_text == "extern" && !matches!(second_token_text, "crate" | "]" | ")") +} + +/// Returns the next relevant token according to the condition given and the offset to it +fn next_token( + iter: &mut impl Iterator, + skip_condition: F, +) -> (Option, usize) +where + F: Fn(TokenKind) -> bool, +{ + let mut skipped_offset = 0; + + for next in iter.by_ref() { + if skip_condition(next.kind) { + skipped_offset += usize::try_from(next.len).unwrap(); + } else { + return (Some(next), skipped_offset); + } + } + (None, skipped_offset) +} + +fn add_location( + source: &str, + path: &Path, + locations: &mut Vec, + token_end_offset: usize, + token_length: usize, +) { + let column = source[..token_end_offset] + .lines() + .last() + .map(|line| (line.len() - token_length + 1) as u32) + .unwrap_or(1); + let line = 1.max(source[..token_end_offset].lines().count() as u32); + locations.push(SourceLocation::new(path, line, Some(column))); +} diff --git a/src/main.rs b/src/main.rs index 04db2ce..39100a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,6 +18,7 @@ mod crate_index; mod demangle; mod deps; pub(crate) mod events; +mod extern_checker; pub(crate) mod fs; pub(crate) mod link_info; pub(crate) mod location; diff --git a/src/problem.rs b/src/problem.rs index f1b8e62..98fecf3 100644 --- a/src/problem.rs +++ b/src/problem.rs @@ -12,6 +12,7 @@ use crate::crate_index::CrateSel; use crate::crate_index::PackageId; use crate::names::SymbolOrDebugName; use crate::proxy::rpc::BinExecutionOutput; +use crate::proxy::rpc::ExternUsage; use crate::proxy::rpc::UnsafeUsage; use crate::symbol::Symbol; use std::collections::BTreeMap; @@ -32,6 +33,7 @@ pub(crate) enum Problem { MissingConfiguration(PathBuf), UsesBuildScript(PackageId), DisallowedUnsafe(UnsafeUsage), + DisallowedExtern(ExternUsage), IsProcMacro(PackageId), DisallowedApiUsage(ApiUsages), OffTreeApiUsage(OffTreeApiUsage), @@ -175,7 +177,9 @@ impl Problem { fn should_send_retry_to_subprocess(&self) -> bool { matches!( self, - &Problem::ExecutionFailed(..) | &Problem::DisallowedUnsafe(..) + &Problem::ExecutionFailed(..) + | &Problem::DisallowedUnsafe(..) + | &Problem::DisallowedExtern(..) ) } @@ -213,6 +217,7 @@ impl Problem { Problem::MissingConfiguration(_) => None, Problem::UsesBuildScript(pkg_id) => Some(pkg_id), Problem::DisallowedUnsafe(d) => Some(d.crate_sel.pkg_id()), + Problem::DisallowedExtern(d) => Some(d.crate_sel.pkg_id()), Problem::IsProcMacro(pkg_id) => Some(pkg_id), Problem::DisallowedApiUsage(d) => Some(&d.pkg_id), Problem::OffTreeApiUsage(d) => Some(&d.usages.pkg_id), @@ -252,6 +257,15 @@ impl Display for Problem { } } } + Problem::DisallowedExtern(usage) => { + write!(f, "`{}` uses extern", usage.crate_sel)?; + if f.alternate() { + writeln!(f)?; + for location in &usage.locations { + writeln!(f, "{location}")?; + } + } + } Problem::UsesBuildScript(pkg_id) => { write!( f, diff --git a/src/proxy/errors.rs b/src/proxy/errors.rs index 3770c43..a719b64 100644 --- a/src/proxy/errors.rs +++ b/src/proxy/errors.rs @@ -6,7 +6,7 @@ use anyhow::Result; use serde::Deserialize; use std::path::Path; -/// Returns source locations for all errors related to use of unsafe code in `output`, which should +/// Returns source locations for all errors related to use of unsafe code except extern in `output`, which should /// be the output from rustc with --error-format=json. pub(crate) fn get_disallowed_unsafe_locations( rustc_output: &std::process::Output, @@ -16,6 +16,16 @@ pub(crate) fn get_disallowed_unsafe_locations( Ok(get_disallowed_unsafe_locations_str(stderr)) } +/// Returns source locations for all errors related to use of exterb code in `output`, which should +/// be the output from rustc with --error-format=json. +pub(crate) fn get_disallowed_extern_locations( + rustc_output: &std::process::Output, +) -> Result> { + let stderr = + std::str::from_utf8(&rustc_output.stderr).context("rustc emitted invalid UTF-8")?; + Ok(get_disallowed_extern_locations_str(stderr)) +} + fn get_disallowed_unsafe_locations_str(output: &str) -> Vec { let mut locations = Vec::new(); //let workspace_root = PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR").unwrap_or_default()); @@ -25,6 +35,30 @@ fn get_disallowed_unsafe_locations_str(output: &str) -> Vec { }; if message.level == "error" && message.code.code == "unsafe_code" + && !message.message.contains("extern") + && let Some(first_span) = message.spans.first() + { + let filename = Path::new(&first_span.file_name); + locations.push(SourceLocation::new( + std::fs::canonicalize(filename).unwrap_or_else(|_| filename.to_owned()), + first_span.line_start, + Some(first_span.column_start), + )); + } + } + locations +} + +fn get_disallowed_extern_locations_str(output: &str) -> Vec { + let mut locations = Vec::new(); + //let workspace_root = PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR").unwrap_or_default()); + for line in output.lines() { + let Ok(message) = serde_json::from_str::(line) else { + continue; + }; + if message.level == "error" + && (message.code.code == "missing_unsafe_on_extern" + || (message.code.code == "unsafe_code" && message.message.contains("extern"))) && let Some(first_span) = message.spans.first() { let filename = Path::new(&first_span.file_name); @@ -40,6 +74,7 @@ fn get_disallowed_unsafe_locations_str(output: &str) -> Vec { #[derive(Deserialize, PartialEq, Eq, Debug)] struct Message { + message: String, code: Code, level: String, spans: Vec, @@ -69,6 +104,7 @@ mod tests { #[test] fn test_unsafe_error() { let json = r#"{ + "message": "usage of an `unsafe` block", "code": {"code": "unsafe_code"}, "level": "error", "spans": [ diff --git a/src/proxy/rpc.rs b/src/proxy/rpc.rs index 6489447..57cb055 100644 --- a/src/proxy/rpc.rs +++ b/src/proxy/rpc.rs @@ -25,7 +25,7 @@ impl RpcClient { RpcClient { socket_path } } - /// Advises the parent process that the specified crate uses unsafe. + /// Advises the parent process that the specified crate uses unsafe, except if followed by extern. pub(crate) fn crate_uses_unsafe( &self, crate_sel: &CrateSel, @@ -40,6 +40,21 @@ impl RpcClient { read_from_stream(&mut ipc) } + /// Advises the parent process that the specified crate uses extern. + pub(crate) fn crate_uses_extern( + &self, + crate_sel: &CrateSel, + locations: Vec, + ) -> Result { + let mut ipc = self.connect()?; + let request = Request::CrateUsesExtern(ExternUsage { + crate_sel: crate_sel.clone(), + locations, + }); + write_to_stream(&request, &mut ipc)?; + read_from_stream(&mut ipc) + } + pub(crate) fn rustc_started(&self, crate_sel: &CrateSel) -> Result { let mut ipc = self.connect()?; let request = Request::RustcStarted(crate_sel.clone()); @@ -80,8 +95,10 @@ impl RpcClient { #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] pub(crate) enum Request { - /// Advises that the specified crate failed to compile because it uses unsafe. + /// Advises that the specified crate failed to compile because it uses unsafe, except if followed by extern. CrateUsesUnsafe(UnsafeUsage), + /// Advises that the specified crate failed to compile because it uses extern. + CrateUsesExtern(ExternUsage), LinkerInvoked(LinkInfo), BinExecutionComplete(BinExecutionOutput), RustcStarted(CrateSel), @@ -114,6 +131,12 @@ pub(crate) struct UnsafeUsage { pub(crate) locations: Vec, } +#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone, Hash)] +pub(crate) struct ExternUsage { + pub(crate) crate_sel: CrateSel, + pub(crate) locations: Vec, +} + /// Writes `value` to `stream`. The format used is the length followed by `value` serialised as /// JSON. pub(crate) fn write_to_stream(value: &T, stream: &mut impl Write) -> Result<()> { diff --git a/src/proxy/subprocess.rs b/src/proxy/subprocess.rs index d9caa99..22f8219 100644 --- a/src/proxy/subprocess.rs +++ b/src/proxy/subprocess.rs @@ -4,6 +4,7 @@ use super::CONFIG_PATH_ENV; use super::ExitCode; use super::cackle_exe; +use super::errors::get_disallowed_extern_locations; use super::errors::get_disallowed_unsafe_locations; use super::rpc::BinExecutionOutput; use super::rpc::RustcOutput; @@ -14,6 +15,7 @@ use crate::config::permissions::PermSel; use crate::config::permissions::Permissions; use crate::crate_index::CrateKind; use crate::crate_index::CrateSel; +use crate::extern_checker; use crate::link_info::LinkInfo; use crate::location::SourceLocation; use crate::outcome::Outcome; @@ -247,7 +249,10 @@ impl RustcRunner { let unsafe_permitted = config .permissions .unsafe_permitted_for_crate(&self.crate_sel); - let mut command = self.get_command(unsafe_permitted)?; + let extern_permitted = config + .permissions + .extern_permitted_for_crate(&self.crate_sel); + let mut command = self.get_command(unsafe_permitted, extern_permitted)?; let output = match crate::sandbox::for_rustc( &config.rustc, &RustcSandboxInputs::from_env(&self.crate_sel)?, @@ -259,6 +264,7 @@ impl RustcRunner { None => command.output()?, }; let mut unsafe_locations = Vec::new(); + let mut extern_locations = Vec::new(); if output.status.code() == Some(0) { let source_paths = crate::deps::source_files_from_rustc_args(std::env::args())?; @@ -274,8 +280,22 @@ impl RustcRunner { if !unsafe_permitted { unsafe_locations.extend(find_unsafe_in_sources(&source_paths)?); } + if !extern_permitted { + extern_locations.extend(find_extern_in_sources(&source_paths)?); + } } else { unsafe_locations.extend(get_disallowed_unsafe_locations(&output)?); + extern_locations.extend(get_disallowed_extern_locations(&output)?); + } + if !extern_locations.is_empty() { + extern_locations.sort(); + extern_locations.dedup(); + let response = rpc_client.crate_uses_extern(&self.crate_sel, extern_locations)?; + if response == Outcome::Continue { + return Ok(RustcRunStatus::Retry); + } else { + return Ok(RustcRunStatus::GiveUp); + } } if !unsafe_locations.is_empty() { unsafe_locations.sort(); @@ -291,7 +311,7 @@ impl RustcRunner { Ok(RustcRunStatus::Done(output)) } - fn get_command(&self, unsafe_permitted: bool) -> Result { + fn get_command(&self, unsafe_permitted: bool, extern_permitted: bool) -> Result { let mut args = std::env::args().skip(2).peekable(); let mut command = Command::new(rustc_path_from_env()?); let mut linker_arg = OsString::new(); @@ -336,11 +356,15 @@ impl RustcRunner { if !unsafe_permitted { command.arg("-Funsafe-code"); } + // for code that is not yet ported to Rust 2024 + if !extern_permitted { + command.arg("-Fmissing-unsafe-on-extern"); + } Ok(command) } } -/// Searches for the unsafe keyword in the specified paths. +/// Searches for the unsafe keyword in the specified paths, unless followed by extern fn find_unsafe_in_sources(paths: &[PathBuf]) -> Result> { let mut locations = Vec::new(); for file in paths { @@ -349,6 +373,15 @@ fn find_unsafe_in_sources(paths: &[PathBuf]) -> Result> { Ok(locations) } +/// Searches for the extern blocks in the specified paths. +fn find_extern_in_sources(paths: &[PathBuf]) -> Result> { + let mut locations = Vec::new(); + for file in paths { + locations.append(&mut extern_checker::scan_path(file)?); + } + Ok(locations) +} + /// Runs the real linker, then advises our parent process of all input files to the linker as well /// as the output file. If the parent process says that all checks have been satisfied, then we /// return, otherwise we exit. diff --git a/src/summary.rs b/src/summary.rs index 1d1ae8e..104c23c 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -91,6 +91,9 @@ impl Summary { if pkg_config.allow_unsafe { permissions.push(format!("unsafe{suffix}")); } + if pkg_config.allow_extern { + permissions.push(format!("extern{suffix}")); + } for api in &pkg_config.allow_apis { permissions.push(format!("{api}{suffix}")); } diff --git a/src/ui/full_term/problems_ui.rs b/src/ui/full_term/problems_ui.rs index cfd895e..9885993 100644 --- a/src/ui/full_term/problems_ui.rs +++ b/src/ui/full_term/problems_ui.rs @@ -816,7 +816,7 @@ fn render_help(f: &mut Frame, mode: Option<&Mode>) { ("f", "Show available automatic fixes for this problem"), ( "d", - "Select and show details of each usage (API/unsafe only)", + "Select and show details of each usage (API/unsafe/extern only)", ), ("t", "Show tree of crate dependencies to this crate"), ("up", "Select previous problem"), @@ -828,7 +828,7 @@ fn render_help(f: &mut Frame, mode: Option<&Mode>) { title = "Help for select-edit"; keys.extend([ ("space/enter/f", "Apply this edit"), - ("d", "Jump to usage details (API/unsafe only)"), + ("d", "Jump to usage details (API/unsafe/extern only)"), ("c", "Add comment to edit (supported edits only)"), ("up", "Select previous edit"), ("down", "Select next edit"), @@ -946,6 +946,17 @@ fn usages_for_problem( })); } } + Some((_, Problem::DisallowedExtern(extern_usage))) => { + for location in &extern_usage.locations { + let pkg_dir = crate_index + .pkg_dir(extern_usage.crate_sel.pkg_id()) + .map(|pkg_dir| pkg_dir.to_owned()); + usages_out.push(Box::new(ExternLocation { + source_location: location.clone(), + pkg_dir, + })); + } + } _ => (), } usages_out.sort_by_key(|u| u.source_location().clone()); @@ -957,6 +968,11 @@ struct UnsafeLocation { pkg_dir: Option, } +struct ExternLocation { + source_location: SourceLocation, + pkg_dir: Option, +} + /// A trait implemented for things that can display in a usage list. trait DisplayUsage { fn source_location(&self) -> &SourceLocation; @@ -1025,11 +1041,30 @@ impl DisplayUsage for UnsafeLocation { } } +impl DisplayUsage for ExternLocation { + fn source_location(&self) -> &SourceLocation { + &self.source_location + } + + fn list_display(&self) -> String { + // In the list, we'd prefer to display source filenames relative to the package root where + // possible. We already know the crate name and all the usage locations for a crate will + // generally be under the package root. For any that aren't, we fall back to using the full + // filename. + let filename = self + .pkg_dir + .as_ref() + .and_then(|pkg_dir| self.source_location.filename().strip_prefix(pkg_dir).ok()) + .unwrap_or_else(|| self.source_location.filename()); + format!("{}:{}", filename.display(), self.source_location.line()) + } +} + fn problem_details(problem: &Problem) -> String { match problem { - Problem::DisallowedUnsafe(..) | Problem::DisallowedApiUsage(..) => { - "Press 'd' to see details of each usage".to_owned() - } + Problem::DisallowedUnsafe(..) + | Problem::DisallowedExtern(..) + | Problem::DisallowedApiUsage(..) => "Press 'd' to see details of each usage".to_owned(), Problem::MissingConfiguration(..) => { "This user interface can guide you through creating an initial cackle.toml. \ Press 'h' at any time to see what keys are available." diff --git a/src/unsafe_checker.rs b/src/unsafe_checker.rs index e94eba2..bc1a43d 100644 --- a/src/unsafe_checker.rs +++ b/src/unsafe_checker.rs @@ -5,9 +5,11 @@ use crate::location::SourceLocation; use anyhow::Context; use anyhow::Result; +use ra_ap_rustc_lexer::Token; +use ra_ap_rustc_lexer::TokenKind; use std::path::Path; -/// Returns the locations of all unsafe usages found in `path`. +/// Returns the locations of all unsafe usages found in `path`, except if followed by extern pub(crate) fn scan_path(path: &Path) -> Result> { let bytes = std::fs::read(path).with_context(|| format!("Failed to read `{}`", path.display()))?; @@ -20,25 +22,119 @@ pub(crate) fn scan_path(path: &Path) -> Result> { } fn scan_string(source: &str, path: &Path) -> Vec { - let mut offset = 0; + let mut token_start_offset = 0; let mut locations = Vec::new(); - for token in ra_ap_rustc_lexer::tokenize(source, ra_ap_rustc_lexer::FrontmatterAllowed::No) { - let new_offset = offset + usize::try_from(token.len).unwrap(); - let token_text = &source[offset..new_offset]; - if token_text == "unsafe" { - let column = source[..new_offset] - .lines() - .last() - .map(|line| (line.len() - token_text.len() + 1) as u32) - .unwrap_or(1); - let line = 1.max(source[..new_offset].lines().count() as u32); - locations.push(SourceLocation::new(path, line, Some(column))); + + let skip_condition = |x| { + matches!( + x, + TokenKind::BlockComment { + doc_style: _, + terminated: _ + } | TokenKind::LineComment { doc_style: _ } + | TokenKind::Whitespace + ) + }; + + let mut iter = ra_ap_rustc_lexer::tokenize(source, ra_ap_rustc_lexer::FrontmatterAllowed::No); + let mut previous_token_text = ""; + let mut previous_token_end_offset = 0; + let mut previous_token_length = 0; + + while let (Some(token), skipped_offset) = next_token(&mut iter, skip_condition) { + let token_length = usize::try_from(token.len).unwrap(); + + token_start_offset += skipped_offset; + + let token_end_offset = token_start_offset + token_length; + let token_text = &source[token_start_offset..token_end_offset]; + if !previous_token_text.is_empty() { + // check against previous token + if check_tokens(previous_token_text, token_text) { + add_location( + source, + path, + &mut locations, + previous_token_end_offset, + previous_token_length, + ); + } + } + previous_token_text = token_text; + previous_token_end_offset = token_end_offset; + previous_token_length = token_length; + // always check against potential future token + if let (Some(next_token), skipped_offset) = next_token(&mut iter, skip_condition) { + // the next token starts after the end of the current token + token_start_offset += token_length; + token_start_offset += skipped_offset; + + let next_token_length = usize::try_from(next_token.len).unwrap(); + + let next_token_end_offset = token_start_offset + next_token_length; + let next_token_text = &source[token_start_offset..next_token_end_offset]; + if check_tokens(token_text, next_token_text) { + add_location(source, path, &mut locations, token_end_offset, token_length); + } + token_start_offset = next_token_end_offset; + // as we consumed the next token already, this will be our previous token in the next loop iteration + previous_token_text = next_token_text; + previous_token_end_offset = next_token_end_offset; + previous_token_length = next_token_length; + } else { + // current token is last token in file + // this should never be valid code, but we flag it nevertheless to be safe + if token_text == "unsafe" { + add_location(source, path, &mut locations, token_end_offset, token_length); + } + // as there should not be another loop iteration this should be useless + // we do it just for completeness + token_start_offset = token_end_offset; } - offset = new_offset; } locations } +fn check_tokens(first_token_text: &str, second_token_text: &str) -> bool { + first_token_text == "unsafe" && second_token_text != "extern" +} + +/// Returns the next relevant token according to the condition given and the offset to it +fn next_token( + iter: &mut impl Iterator, + skip_condition: F, +) -> (Option, usize) +where + F: Fn(TokenKind) -> bool, +{ + let mut skipped_offset = 0; + + for next in iter.by_ref() { + if skip_condition(next.kind) { + skipped_offset += usize::try_from(next.len).unwrap(); + } else { + return (Some(next), skipped_offset); + } + } + (None, skipped_offset) +} + +fn add_location( + source: &str, + path: &Path, + locations: &mut Vec, + token_end_offset: usize, + token_length: usize, +) { + let column = source[..token_end_offset] + .lines() + .last() + .map(|line| (line.len() - token_length + 1) as u32) + .unwrap_or(1); + let line = 1.max(source[..token_end_offset].lines().count() as u32); + locations.push(SourceLocation::new(path, line, Some(column))); +} + #[cfg(test)] mod tests { use crate::unsafe_checker::scan_path; From a77e400d6d731182351c425fec8c461bda5f749f Mon Sep 17 00:00:00 2001 From: XSpielinbox <55600187+xspielinbox@users.noreply.github.com> Date: Wed, 13 May 2026 22:17:02 +0200 Subject: [PATCH 2/5] Update cackle.toml according to new permission --- cackle.toml | 7 +++++++ test_crates/cackle.toml | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/cackle.toml b/cackle.toml index 21d81da..5b95c35 100644 --- a/cackle.toml +++ b/cackle.toml @@ -82,6 +82,7 @@ allow_unsafe = true build.allow_apis = [ "process", ] +allow_extern = true [pkg.proc-macro2] allow_unsafe = true @@ -102,6 +103,7 @@ allow_unsafe = true [pkg.linux-raw-sys] allow_unsafe = true +allow_extern = true [pkg.hashbrown] allow_unsafe = true @@ -129,6 +131,7 @@ allow_unsafe = true [pkg.signal-hook-registry] allow_unsafe = true +allow_extern = true [pkg.syn] allow_unsafe = true @@ -193,6 +196,7 @@ build.allow_apis = [ "fs", "process", ] +allow_extern = true [pkg.twox-hash] allow_unsafe = true @@ -297,6 +301,7 @@ allow_unsafe = true [pkg.getrandom] allow_unsafe = true +allow_extern = true [pkg.serde_core] allow_unsafe = true @@ -326,6 +331,7 @@ allow_unsafe = true [pkg.errno] allow_unsafe = true +allow_extern = true [pkg.toml_parser] allow_unsafe = true @@ -385,3 +391,4 @@ allow_unsafe = true [pkg.time] allow_unsafe = true +allow_extern = true diff --git a/test_crates/cackle.toml b/test_crates/cackle.toml index ee9c435..7316645 100644 --- a/test_crates/cackle.toml +++ b/test_crates/cackle.toml @@ -54,6 +54,7 @@ build.sandbox.allow_network = true from.test.allow_apis = [ "unix_sockets", ] +allow_extern = true [pkg.crab-2] allow_apis = [ @@ -68,6 +69,7 @@ build.allow_build_instructions = [ "cargo:rustc-link-*" ] build.allow_unsafe = true +allow_extern = true [pkg.crab-3] allow_unsafe = true @@ -120,6 +122,7 @@ allow_apis = [ "crab-1::fs", "env", ] +allow_extern = true [pkg.crab-7] allow_unsafe = true @@ -127,6 +130,7 @@ allow_apis = [ "env", "fs", ] +allow_extern = true [pkg.crab-8] allow_apis = [ @@ -159,6 +163,7 @@ build.allow_apis = [ allow_build_instructions = [ "cargo:rustc-link-*", ] +allow_extern = true [pkg.crab-11] test.sandbox.kind = "Disabled" From 9fc67ae45e848d1ae9eaf42db01fa2555dd7d4a3 Mon Sep 17 00:00:00 2001 From: XSpielinbox <55600187+xspielinbox@users.noreply.github.com> Date: Wed, 13 May 2026 22:18:21 +0200 Subject: [PATCH 3/5] Introduce new config file format version --- cackle.toml | 2 +- src/config/versions.rs | 6 ++++++ test_crates/cackle.toml | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cackle.toml b/cackle.toml index 5b95c35..eb889d3 100644 --- a/cackle.toml +++ b/cackle.toml @@ -1,5 +1,5 @@ [common] -version = 2 +version = 3 import_std = [ "fs", "net", diff --git a/src/config/versions.rs b/src/config/versions.rs index d238ac5..fd708bf 100644 --- a/src/config/versions.rs +++ b/src/config/versions.rs @@ -54,6 +54,12 @@ pub(crate) const VERSIONS: &[Version] = &[ Ok(()) }, }, + Version { + number: 3, + change_notes: "Added field allow_extern", + apply_fn: |_| {}, + update_fn: |_| Ok(()), + }, ]; impl Version { diff --git a/test_crates/cackle.toml b/test_crates/cackle.toml index 7316645..853e15c 100644 --- a/test_crates/cackle.toml +++ b/test_crates/cackle.toml @@ -1,5 +1,5 @@ [common] -version = 2 +version = 3 features = [ "crash-if-not-sandboxed", "foo", From 2026edb77e29d8b6f6a3c3727276c554e012e3b5 Mon Sep 17 00:00:00 2001 From: XSpielinbox <55600187+xspielinbox@users.noreply.github.com> Date: Thu, 14 May 2026 21:32:53 +0200 Subject: [PATCH 4/5] Revert changes to main cackle.toml --- cackle.toml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/cackle.toml b/cackle.toml index eb889d3..21d81da 100644 --- a/cackle.toml +++ b/cackle.toml @@ -1,5 +1,5 @@ [common] -version = 3 +version = 2 import_std = [ "fs", "net", @@ -82,7 +82,6 @@ allow_unsafe = true build.allow_apis = [ "process", ] -allow_extern = true [pkg.proc-macro2] allow_unsafe = true @@ -103,7 +102,6 @@ allow_unsafe = true [pkg.linux-raw-sys] allow_unsafe = true -allow_extern = true [pkg.hashbrown] allow_unsafe = true @@ -131,7 +129,6 @@ allow_unsafe = true [pkg.signal-hook-registry] allow_unsafe = true -allow_extern = true [pkg.syn] allow_unsafe = true @@ -196,7 +193,6 @@ build.allow_apis = [ "fs", "process", ] -allow_extern = true [pkg.twox-hash] allow_unsafe = true @@ -301,7 +297,6 @@ allow_unsafe = true [pkg.getrandom] allow_unsafe = true -allow_extern = true [pkg.serde_core] allow_unsafe = true @@ -331,7 +326,6 @@ allow_unsafe = true [pkg.errno] allow_unsafe = true -allow_extern = true [pkg.toml_parser] allow_unsafe = true @@ -391,4 +385,3 @@ allow_unsafe = true [pkg.time] allow_unsafe = true -allow_extern = true From 6897b9b10b114ced976e3231e669f635d1f65eb0 Mon Sep 17 00:00:00 2001 From: XSpielinbox <55600187+xspielinbox@users.noreply.github.com> Date: Fri, 15 May 2026 11:45:29 +0200 Subject: [PATCH 5/5] Add version 2 permission fallback --- src/config/permissions.rs | 20 ++++++++++++++++---- src/proxy/subprocess.rs | 5 ++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/config/permissions.rs b/src/config/permissions.rs index 0797dde..470a599 100644 --- a/src/config/permissions.rs +++ b/src/config/permissions.rs @@ -2,6 +2,7 @@ use super::PackageConfig; use super::PackageName; use super::RawConfig; use super::SandboxConfig; +use crate::config::CommonConfig; use crate::crate_index::CrateIndex; use crate::crate_index::CrateKind; use crate::crate_index::CrateSel; @@ -121,10 +122,21 @@ impl Permissions { .is_some_and(|crate_config| crate_config.allow_unsafe) } - pub(crate) fn extern_permitted_for_crate(&self, crate_sel: &CrateSel) -> bool { - self.packages - .get(&PermSel::for_non_build_output(crate_sel)) - .is_some_and(|crate_config| crate_config.allow_extern) + pub(crate) fn extern_permitted_for_crate( + &self, + crate_sel: &CrateSel, + common_config: &CommonConfig, + ) -> bool { + if common_config.version <= 2 { + // For compatibility treat extern permission as granted when unsafe is granted + self.packages + .get(&PermSel::for_non_build_output(crate_sel)) + .is_some_and(|crate_config| crate_config.allow_unsafe) + } else { + self.packages + .get(&PermSel::for_non_build_output(crate_sel)) + .is_some_and(|crate_config| crate_config.allow_extern) + } } pub(crate) fn get(&self, perm_sel: &PermSel) -> Option<&PackageConfig> { diff --git a/src/proxy/subprocess.rs b/src/proxy/subprocess.rs index 22f8219..601c750 100644 --- a/src/proxy/subprocess.rs +++ b/src/proxy/subprocess.rs @@ -9,6 +9,7 @@ use super::errors::get_disallowed_unsafe_locations; use super::rpc::BinExecutionOutput; use super::rpc::RustcOutput; use super::run_command; +use crate::config::CommonConfig; use crate::config::Config; use crate::config::RustcConfig; use crate::config::permissions::PermSel; @@ -251,7 +252,7 @@ impl RustcRunner { .unsafe_permitted_for_crate(&self.crate_sel); let extern_permitted = config .permissions - .extern_permitted_for_crate(&self.crate_sel); + .extern_permitted_for_crate(&self.crate_sel, &config.common); let mut command = self.get_command(unsafe_permitted, extern_permitted)?; let output = match crate::sandbox::for_rustc( &config.rustc, @@ -421,6 +422,7 @@ fn default_linker() -> String { #[derive(Deserialize, Serialize, PartialEq, Eq, Debug)] pub(crate) struct SubprocessConfig { + common: CommonConfig, permissions: Permissions, rustc: RustcConfig, } @@ -435,6 +437,7 @@ impl SubprocessConfig { pub(crate) fn from_full_config(full_config: &Config) -> Self { Self { + common: full_config.raw.common.clone(), permissions: full_config.permissions.clone(), rustc: full_config.raw.rustc.clone(), }