From ffb4f4ba8355df2b96266791457adaee95c1ff37 Mon Sep 17 00:00:00 2001 From: Daniel Veljjanoski Date: Thu, 15 Oct 2026 09:20:11 +0200 Subject: [PATCH 1/3] codegen: escape companion include! names, and reject a path in release builds too --- .../unreleased/fixed-20261015-083325.yaml | 3 + buffa-codegen/src/lib.rs | 25 +++-- buffa-codegen/src/tests/generation.rs | 98 ++++++++++++++++++- 3 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 .changes/unreleased/fixed-20261015-083325.yaml diff --git a/.changes/unreleased/fixed-20261015-083325.yaml b/.changes/unreleased/fixed-20261015-083325.yaml new file mode 100644 index 000000000..64065417f --- /dev/null +++ b/.changes/unreleased/fixed-20261015-083325.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: '**`apply_companions` writes each companion name into the stitcher as an escaped string literal, and rejects a name with `/` or `\` in release builds too** (#708). The name went into `include!("…")` unescaped, so a name with a `"` gave a stitcher that did not compile or named a different file, and the only check on the name was a `debug_assert!`, which the release build of a code generator skips. The `include!` is now written with an escaped literal, as at buffa''s own include sites. A name that contains `/` or `\` panics in every build, before any output is changed; a generator that used such a name in a release build should switch to a bare name such as `.__myplugin.rs`.' +time: 2026-10-15T08:33:25.768858+02:00 diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index c96285b7f..2b0095a7d 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -5393,32 +5393,41 @@ pub fn proto_path_to_stem(proto_path: &str) -> String { /// /// `name` must be a bare-sibling filename — the same convention buffa uses /// for its own `include!` calls, so it resolves relative to the stitcher -/// without any `OUT_DIR` prefix. Names must not contain `"`, `\`, `/`, or -/// newlines (the function `debug_assert!`s this in debug builds), and must +/// without any `OUT_DIR` prefix. Names must not contain `/` or `\`, and must /// not collide with any of buffa's own generated filenames for the same /// package (`.rs`, `.__view.rs`, etc.) — pick an unused suffix -/// such as `.__myplugin.rs`. +/// such as `.__myplugin.rs`. The name is written into the `include!` +/// as an escaped string literal, as buffa's own include paths are. /// /// Companion files with no matching `PackageMod` (e.g. for a package buffa /// did not generate any output for) are still appended to `files` but no /// `include!` is emitted; the caller is responsible for wiring them up. If /// you don't expect orphans, check that every companion's `package` appears /// in `files` as a `PackageMod` after calling. +/// +/// # Panics +/// +/// Panics if a companion's `name` contains `/` or `\`. Every name is checked +/// before `files` is changed. +#[track_caller] pub fn apply_companions(files: &mut Vec, companions: Vec) { for comp in &companions { - debug_assert!( - !comp.name.contains(['"', '\\', '/', '\n']), - "companion file name {:?} contains a character that would break \ - the generated include!() literal or its bare-sibling resolution", + assert!( + !comp.name.contains(['/', '\\']), + "companion file name {:?} contains a path separator ('/' or '\\'); use a \ + bare file name such as `.__myplugin.rs`, which include!() resolves \ + next to the stitcher", comp.name ); + } + for comp in &companions { if let Some(pkg_mod) = files .iter_mut() .find(|f| f.kind == GeneratedFileKind::PackageMod && f.package == comp.package) { pkg_mod .content - .push_str(&format!("include!(\"{}\");\n", comp.name)); + .push_str(&format!("include!({:?});\n", comp.name)); } } files.extend(companions); diff --git a/buffa-codegen/src/tests/generation.rs b/buffa-codegen/src/tests/generation.rs index c54456829..3c865f75c 100644 --- a/buffa-codegen/src/tests/generation.rs +++ b/buffa-codegen/src/tests/generation.rs @@ -2898,14 +2898,59 @@ fn apply_companions_empty_is_noop() { } #[test] -#[cfg(debug_assertions)] -#[should_panic(expected = "contains a character that would break")] -fn apply_companions_rejects_unsafe_name() { +fn apply_companions_escapes_name_in_include() { + // The include! must name exactly this file, as buffa's own include sites + // do: the quote must not end the string literal early, and the control + // and bidi characters must be escaped. + let mut file = proto3_file("svc/echo.proto"); + file.package = Some("svc".to_string()); + let mut files = generate( + &[file], + &["svc/echo.proto".to_string()], + &CodeGenConfig::default(), + ) + .expect("generate ok"); + + let name = "svc.we\"ird'\n\t\u{301}\u{202e}.__service.rs"; + let companion = GeneratedFile { + name: name.to_string(), + package: "svc".to_string(), + kind: GeneratedFileKind::Companion, + content: "pub struct EchoService;".to_string(), + }; + apply_companions(&mut files, vec![companion]); + + let stitcher = files + .iter() + .find(|f| f.kind == GeneratedFileKind::PackageMod) + .expect("stitcher present"); + let parsed = syn::parse_file(&stitcher.content).expect("stitcher still parses"); + let included: Vec = parsed + .items + .iter() + .filter_map(|item| match item { + syn::Item::Macro(m) if m.mac.path.is_ident("include") => m + .mac + .parse_body::() + .ok() + .map(|lit| lit.value()), + _ => None, + }) + .collect(); + assert!( + included.iter().any(|path| path == name), + "no include! names {name:?}; included: {included:?}" + ); +} + +#[test] +#[should_panic(expected = "contains a path separator")] +fn apply_companions_rejects_name_with_slash() { let mut files: Vec = vec![]; apply_companions( &mut files, vec![GeneratedFile { - name: r#"bad".rs"#.to_string(), + name: "sub/bad.rs".to_string(), package: "svc".to_string(), kind: GeneratedFileKind::Companion, content: String::new(), @@ -2913,6 +2958,51 @@ fn apply_companions_rejects_unsafe_name() { ); } +#[test] +#[should_panic(expected = "contains a path separator")] +fn apply_companions_rejects_name_with_backslash() { + let mut files: Vec = vec![]; + apply_companions( + &mut files, + vec![GeneratedFile { + name: r"sub\bad.rs".to_string(), + package: "svc".to_string(), + kind: GeneratedFileKind::Companion, + content: String::new(), + }], + ); +} + +#[test] +fn apply_companions_checks_every_name_before_changing_files() { + // The bad name is second: the first companion's include! must not be + // written before the panic. + let mut file = proto3_file("svc/echo.proto"); + file.package = Some("svc".to_string()); + let mut files = generate( + &[file], + &["svc/echo.proto".to_string()], + &CodeGenConfig::default(), + ) + .expect("generate ok"); + let before: Vec = files.iter().map(|f| f.content.clone()).collect(); + + let companion = |name: &str| GeneratedFile { + name: name.to_string(), + package: "svc".to_string(), + kind: GeneratedFileKind::Companion, + content: String::new(), + }; + let companions = vec![companion("svc.echo.__good.rs"), companion("sub/bad.rs")]; + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + apply_companions(&mut files, companions) + })); + + assert!(result.is_err(), "a name with '/' must panic"); + let after: Vec = files.iter().map(|f| f.content.clone()).collect(); + assert_eq!(before, after, "files changed before the panic"); +} + #[test] fn apply_companions_file_per_package() { let mut file = proto3_file("svc/msg.proto"); From 9c80178ab70ee06daecb35ee2675355b3c0e998a Mon Sep 17 00:00:00 2001 From: Daniel Veljjanoski Date: Thu, 15 Oct 2026 10:10:12 +0200 Subject: [PATCH 2/3] changelog: cite the PR number --- .changes/unreleased/fixed-20261015-083325.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/fixed-20261015-083325.yaml b/.changes/unreleased/fixed-20261015-083325.yaml index 64065417f..f9b82a7a8 100644 --- a/.changes/unreleased/fixed-20261015-083325.yaml +++ b/.changes/unreleased/fixed-20261015-083325.yaml @@ -1,3 +1,3 @@ kind: Fixed -body: '**`apply_companions` writes each companion name into the stitcher as an escaped string literal, and rejects a name with `/` or `\` in release builds too** (#708). The name went into `include!("…")` unescaped, so a name with a `"` gave a stitcher that did not compile or named a different file, and the only check on the name was a `debug_assert!`, which the release build of a code generator skips. The `include!` is now written with an escaped literal, as at buffa''s own include sites. A name that contains `/` or `\` panics in every build, before any output is changed; a generator that used such a name in a release build should switch to a bare name such as `.__myplugin.rs`.' +body: '**`apply_companions` writes each companion name into the stitcher as an escaped string literal, and rejects a name with `/` or `\` in release builds too** (#718, closes #708). The name went into `include!("…")` unescaped, so a name with a `"` gave a stitcher that did not compile or named a different file, and the only check on the name was a `debug_assert!`, which the release build of a code generator skips. The `include!` is now written with an escaped literal, as at buffa''s own include sites. A name that contains `/` or `\` panics in every build, before any output is changed; a generator that used such a name in a release build should switch to a bare name such as `.__myplugin.rs`.' time: 2026-10-15T08:33:25.768858+02:00 From 11235ce531d5d18d847608f91616484fae0f451e Mon Sep 17 00:00:00 2001 From: Iain McGinniss Date: Fri, 16 Oct 2026 15:47:44 -0700 Subject: [PATCH 3/3] codegen: reject a companion name that names a directory, and pin the escaping test An empty name, `.` and `..` contain no separator, so they passed the check and produced an include! of a directory. apply_companions now panics for them as well, before any output is changed. apply_companions_escapes_name_in_include compared the parsed literal's value with the name. A raw newline, tab or bidi control parses to the same value as its escape, so only the quote pinned the escaping. The test now also compares the appended text with the escaped literal, and the name gains a carriage return, which does not lex unescaped. --- .../unreleased/fixed-20261015-083325.yaml | 2 +- buffa-codegen/src/lib.rs | 21 ++++-- buffa-codegen/src/tests/generation.rs | 68 +++++++++++++++++-- 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/.changes/unreleased/fixed-20261015-083325.yaml b/.changes/unreleased/fixed-20261015-083325.yaml index f9b82a7a8..487525259 100644 --- a/.changes/unreleased/fixed-20261015-083325.yaml +++ b/.changes/unreleased/fixed-20261015-083325.yaml @@ -1,3 +1,3 @@ kind: Fixed -body: '**`apply_companions` writes each companion name into the stitcher as an escaped string literal, and rejects a name with `/` or `\` in release builds too** (#718, closes #708). The name went into `include!("…")` unescaped, so a name with a `"` gave a stitcher that did not compile or named a different file, and the only check on the name was a `debug_assert!`, which the release build of a code generator skips. The `include!` is now written with an escaped literal, as at buffa''s own include sites. A name that contains `/` or `\` panics in every build, before any output is changed; a generator that used such a name in a release build should switch to a bare name such as `.__myplugin.rs`.' +body: '**`apply_companions` writes each companion name into the stitcher as an escaped string literal, and rejects a name with `/` or `\` in release builds too** (#718, closes #708). The name went into `include!("…")` unescaped, so a name with a `"` gave a stitcher that did not compile or named a different file, and the only check on the name was a `debug_assert!`, which the release build of a code generator skips. The `include!` is now written with an escaped literal, as at buffa''s own include sites. A name that contains `/` or `\`, or that is empty, `.` or `..`, panics in every build, before any output is changed; a generator that used such a name in a release build should switch to a bare name such as `.__myplugin.rs`.' time: 2026-10-15T08:33:25.768858+02:00 diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index 2b0095a7d..6beaf7ac4 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -5393,11 +5393,12 @@ pub fn proto_path_to_stem(proto_path: &str) -> String { /// /// `name` must be a bare-sibling filename — the same convention buffa uses /// for its own `include!` calls, so it resolves relative to the stitcher -/// without any `OUT_DIR` prefix. Names must not contain `/` or `\`, and must -/// not collide with any of buffa's own generated filenames for the same -/// package (`.rs`, `.__view.rs`, etc.) — pick an unused suffix -/// such as `.__myplugin.rs`. The name is written into the `include!` -/// as an escaped string literal, as buffa's own include paths are. +/// without any `OUT_DIR` prefix. Names must not contain `/` or `\`, must not +/// be empty, `.` or `..`, and must not collide with any of buffa's own +/// generated filenames for the same package (`.rs`, `.__view.rs`, +/// etc.) — pick an unused suffix such as `.__myplugin.rs`. The name is +/// written into the `include!` as an escaped string literal, as buffa's own +/// include paths are. /// /// Companion files with no matching `PackageMod` (e.g. for a package buffa /// did not generate any output for) are still appended to `files` but no @@ -5407,8 +5408,8 @@ pub fn proto_path_to_stem(proto_path: &str) -> String { /// /// # Panics /// -/// Panics if a companion's `name` contains `/` or `\`. Every name is checked -/// before `files` is changed. +/// Panics if a companion's `name` contains `/` or `\`, or is empty, `.` or +/// `..`. Every name is checked before `files` is changed. #[track_caller] pub fn apply_companions(files: &mut Vec, companions: Vec) { for comp in &companions { @@ -5419,6 +5420,12 @@ pub fn apply_companions(files: &mut Vec, companions: Vec.__myplugin.rs`", + comp.name + ); } for comp in &companions { if let Some(pkg_mod) = files diff --git a/buffa-codegen/src/tests/generation.rs b/buffa-codegen/src/tests/generation.rs index 3c865f75c..91a969a43 100644 --- a/buffa-codegen/src/tests/generation.rs +++ b/buffa-codegen/src/tests/generation.rs @@ -2911,20 +2911,26 @@ fn apply_companions_escapes_name_in_include() { ) .expect("generate ok"); - let name = "svc.we\"ird'\n\t\u{301}\u{202e}.__service.rs"; + let name = "svc.we\"ird'\n\r\t\u{301}\u{202e}.__service.rs"; let companion = GeneratedFile { name: name.to_string(), package: "svc".to_string(), kind: GeneratedFileKind::Companion, content: "pub struct EchoService;".to_string(), }; + let stitcher = |files: &[GeneratedFile]| { + files + .iter() + .find(|f| f.kind == GeneratedFileKind::PackageMod) + .expect("stitcher present") + .content + .clone() + }; + let before = stitcher(&files); apply_companions(&mut files, vec![companion]); + let after = stitcher(&files); - let stitcher = files - .iter() - .find(|f| f.kind == GeneratedFileKind::PackageMod) - .expect("stitcher present"); - let parsed = syn::parse_file(&stitcher.content).expect("stitcher still parses"); + let parsed = syn::parse_file(&after).expect("stitcher still parses"); let included: Vec = parsed .items .iter() @@ -2941,6 +2947,56 @@ fn apply_companions_escapes_name_in_include() { included.iter().any(|path| path == name), "no include! names {name:?}; included: {included:?}" ); + // A raw newline, tab or bidi character parses to the same value as its + // escape (a raw carriage return does not lex at all), so the appended + // text is checked as well: it is the escaped literal, and every control + // and non-ASCII character of the name is escaped in it. + let appended = after + .strip_prefix(before.as_str()) + .expect("apply_companions only appends to the stitcher"); + assert_eq!(appended, format!("include!({name:?});\n")); + let raw: Vec = appended + .trim_end_matches('\n') + .chars() + .filter(|c| !c.is_ascii() || c.is_ascii_control()) + .collect(); + assert!(raw.is_empty(), "unescaped in the include line: {raw:?}"); +} + +#[test] +fn apply_companions_rejects_names_that_are_not_files() { + // A good companion comes first: its include! must not be written before + // the panic for the bad name. + for bad in ["", ".", ".."] { + let mut file = proto3_file("svc/echo.proto"); + file.package = Some("svc".to_string()); + let mut files = generate( + &[file], + &["svc/echo.proto".to_string()], + &CodeGenConfig::default(), + ) + .expect("generate ok"); + let before: Vec = files.iter().map(|f| f.content.clone()).collect(); + + let companion = |name: &str| GeneratedFile { + name: name.to_string(), + package: "svc".to_string(), + kind: GeneratedFileKind::Companion, + content: String::new(), + }; + let companions = vec![companion("svc.echo.__good.rs"), companion(bad)]; + let payload = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + apply_companions(&mut files, companions) + })) + .expect_err("must panic"); + + let message = payload + .downcast_ref::() + .expect("the assert! message is a String"); + assert!(message.contains("names a directory"), "{bad:?}: {message}"); + let after: Vec = files.iter().map(|f| f.content.clone()).collect(); + assert_eq!(before, after, "{bad:?}: files changed before the panic"); + } } #[test]