From 3cb9b5669d6f607076c4e22c0c8398a0124521d9 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 8 Sep 2026 22:13:27 -0400 Subject: [PATCH 1/5] Disallow npm-installed pnpm in workflows --- .github/workflows/release.yml | 2 +- tools/ci/commands/lint/src/main.rs | 57 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 53cc7ba0594..876d190a1e2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -328,7 +328,7 @@ jobs: npm --version - name: Install pnpm - run: npm install -g pnpm + uses: ./.github/actions/setup-pnpm - name: Run NPM release (dry-run) if: ${{ inputs.dry_run }} diff --git a/tools/ci/commands/lint/src/main.rs b/tools/ci/commands/lint/src/main.rs index 956d4f44ce4..2f954a23adc 100644 --- a/tools/ci/commands/lint/src/main.rs +++ b/tools/ci/commands/lint/src/main.rs @@ -104,6 +104,30 @@ fn npmrc_minimum_release_age(path: &Path, expected_minimum_release_age: u64) -> }) } +fn shell_line_installs_pnpm_with_npm(line: &str) -> bool { + let line = line + .split_once('#') + .map_or(line, |(line, _comment)| line) + .trim(); + let line = line.strip_prefix("run:").unwrap_or(line).trim(); + let line = line.strip_prefix("-").unwrap_or(line).trim(); + let line = line.trim_matches(|c| c == '"' || c == '\''); + let tokens: Vec<_> = line.split_whitespace().collect(); + + tokens.first() == Some(&"npm") + && tokens + .iter() + .any(|token| *token == "install" || *token == "i") + && tokens.iter().any(|token| { + let token = token.trim_matches(|c: char| c == '"' || c == '\'' || c == ';'); + token == "pnpm" || token.starts_with("pnpm@") + }) +} + +fn workflow_installs_pnpm_with_npm(contents: &str) -> bool { + contents.lines().any(shell_line_installs_pnpm_with_npm) +} + fn check_pnpm_release_age_policy() -> Result<()> { ensure_repo_root()?; @@ -211,11 +235,44 @@ fn check_pnpm_release_age_policy() -> Result<()> { workflow_path.display() ); } + if workflow_installs_pnpm_with_npm(&contents) { + bail!( + "{} must use ./.github/actions/setup-pnpm instead of installing pnpm with npm", + workflow_path.display() + ); + } } Ok(()) } +#[cfg(test)] +mod tests { + use super::workflow_installs_pnpm_with_npm; + + #[test] + fn detects_direct_npm_pnpm_install() { + assert!(workflow_installs_pnpm_with_npm("run: npm install -g pnpm\n")); + assert!(workflow_installs_pnpm_with_npm( + "run: npm i --global pnpm@10.16.0\n" + )); + assert!(workflow_installs_pnpm_with_npm( + "run: |\n npm install --global pnpm\n" + )); + } + + #[test] + fn allows_other_npm_and_pnpm_commands() { + assert!(!workflow_installs_pnpm_with_npm( + "run: npm install --global npm@11.5.1\n" + )); + assert!(!workflow_installs_pnpm_with_npm("run: pnpm install\n")); + assert!(!workflow_installs_pnpm_with_npm( + "uses: ./.github/actions/setup-pnpm\n" + )); + } +} + /// Codex plugin ships a copy of `skills/`, because plugin installers do not follow symlinks, /// this checks if the copy is in sync fn check_codex_plugin_skills_sync() -> Result<()> { From 64a41f91f7bdc1560ba71f512c4051beececbf84 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Wed, 9 Sep 2026 09:07:01 -0400 Subject: [PATCH 2/5] Format CI lint command --- tools/ci/commands/lint/src/main.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/tools/ci/commands/lint/src/main.rs b/tools/ci/commands/lint/src/main.rs index 2f954a23adc..c6e78ce3b91 100644 --- a/tools/ci/commands/lint/src/main.rs +++ b/tools/ci/commands/lint/src/main.rs @@ -105,19 +105,14 @@ fn npmrc_minimum_release_age(path: &Path, expected_minimum_release_age: u64) -> } fn shell_line_installs_pnpm_with_npm(line: &str) -> bool { - let line = line - .split_once('#') - .map_or(line, |(line, _comment)| line) - .trim(); + let line = line.split_once('#').map_or(line, |(line, _comment)| line).trim(); let line = line.strip_prefix("run:").unwrap_or(line).trim(); let line = line.strip_prefix("-").unwrap_or(line).trim(); let line = line.trim_matches(|c| c == '"' || c == '\''); let tokens: Vec<_> = line.split_whitespace().collect(); tokens.first() == Some(&"npm") - && tokens - .iter() - .any(|token| *token == "install" || *token == "i") + && tokens.iter().any(|token| *token == "install" || *token == "i") && tokens.iter().any(|token| { let token = token.trim_matches(|c: char| c == '"' || c == '\'' || c == ';'); token == "pnpm" || token.starts_with("pnpm@") @@ -253,12 +248,8 @@ mod tests { #[test] fn detects_direct_npm_pnpm_install() { assert!(workflow_installs_pnpm_with_npm("run: npm install -g pnpm\n")); - assert!(workflow_installs_pnpm_with_npm( - "run: npm i --global pnpm@10.16.0\n" - )); - assert!(workflow_installs_pnpm_with_npm( - "run: |\n npm install --global pnpm\n" - )); + assert!(workflow_installs_pnpm_with_npm("run: npm i --global pnpm@10.16.0\n")); + assert!(workflow_installs_pnpm_with_npm("run: |\n npm install --global pnpm\n")); } #[test] @@ -267,9 +258,7 @@ mod tests { "run: npm install --global npm@11.5.1\n" )); assert!(!workflow_installs_pnpm_with_npm("run: pnpm install\n")); - assert!(!workflow_installs_pnpm_with_npm( - "uses: ./.github/actions/setup-pnpm\n" - )); + assert!(!workflow_installs_pnpm_with_npm("uses: ./.github/actions/setup-pnpm\n")); } } From 70332f033c9e789d03606240e9653749430d4395 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Wed, 9 Sep 2026 10:17:11 -0400 Subject: [PATCH 3/5] Fix ci-lint test module placement --- tools/ci/commands/lint/src/main.rs | 42 +++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tools/ci/commands/lint/src/main.rs b/tools/ci/commands/lint/src/main.rs index c6e78ce3b91..79f3f2f90ee 100644 --- a/tools/ci/commands/lint/src/main.rs +++ b/tools/ci/commands/lint/src/main.rs @@ -241,27 +241,6 @@ fn check_pnpm_release_age_policy() -> Result<()> { Ok(()) } -#[cfg(test)] -mod tests { - use super::workflow_installs_pnpm_with_npm; - - #[test] - fn detects_direct_npm_pnpm_install() { - assert!(workflow_installs_pnpm_with_npm("run: npm install -g pnpm\n")); - assert!(workflow_installs_pnpm_with_npm("run: npm i --global pnpm@10.16.0\n")); - assert!(workflow_installs_pnpm_with_npm("run: |\n npm install --global pnpm\n")); - } - - #[test] - fn allows_other_npm_and_pnpm_commands() { - assert!(!workflow_installs_pnpm_with_npm( - "run: npm install --global npm@11.5.1\n" - )); - assert!(!workflow_installs_pnpm_with_npm("run: pnpm install\n")); - assert!(!workflow_installs_pnpm_with_npm("uses: ./.github/actions/setup-pnpm\n")); - } -} - /// Codex plugin ships a copy of `skills/`, because plugin installers do not follow symlinks, /// this checks if the copy is in sync fn check_codex_plugin_skills_sync() -> Result<()> { @@ -399,3 +378,24 @@ fn main() -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::workflow_installs_pnpm_with_npm; + + #[test] + fn detects_direct_npm_pnpm_install() { + assert!(workflow_installs_pnpm_with_npm("run: npm install -g pnpm\n")); + assert!(workflow_installs_pnpm_with_npm("run: npm i --global pnpm@10.16.0\n")); + assert!(workflow_installs_pnpm_with_npm("run: |\n npm install --global pnpm\n")); + } + + #[test] + fn allows_other_npm_and_pnpm_commands() { + assert!(!workflow_installs_pnpm_with_npm( + "run: npm install --global npm@11.5.1\n" + )); + assert!(!workflow_installs_pnpm_with_npm("run: pnpm install\n")); + assert!(!workflow_installs_pnpm_with_npm("uses: ./.github/actions/setup-pnpm\n")); + } +} From a7cdbc8ee333a7bd8b139539b5cd0d0970e50d14 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Wed, 9 Sep 2026 07:21:40 -0700 Subject: [PATCH 4/5] [bot/widen-pnpm-workflow-lint]: Use regex for npm pnpm lint --- Cargo.lock | 1 + tools/ci/commands/lint/Cargo.toml | 1 + tools/ci/commands/lint/src/main.rs | 24 +++++++----------------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f9455400ec..fda0e343b52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -969,6 +969,7 @@ dependencies = [ "ci-common", "clap 4.5.50", "duct", + "regex", "serde_json", ] diff --git a/tools/ci/commands/lint/Cargo.toml b/tools/ci/commands/lint/Cargo.toml index 44bfa5776bd..66e7d8a7f9c 100644 --- a/tools/ci/commands/lint/Cargo.toml +++ b/tools/ci/commands/lint/Cargo.toml @@ -7,5 +7,6 @@ edition.workspace = true anyhow.workspace = true clap.workspace = true duct.workspace = true +regex.workspace = true serde_json.workspace = true ci-common = { path = "../../common" } diff --git a/tools/ci/commands/lint/src/main.rs b/tools/ci/commands/lint/src/main.rs index 79f3f2f90ee..c84ca4001e7 100644 --- a/tools/ci/commands/lint/src/main.rs +++ b/tools/ci/commands/lint/src/main.rs @@ -4,12 +4,14 @@ use anyhow::{bail, Context, Result}; use ci_common::{ensure_repo_root, pnpm}; use clap::Parser; use duct::cmd; +use regex::Regex; use serde_json::Value; use std::collections::BTreeSet; use std::ffi::OsString; use std::fs; use std::path::Path; use std::path::PathBuf; +use std::sync::LazyLock; /// Lints the codebase /// @@ -104,23 +106,9 @@ fn npmrc_minimum_release_age(path: &Path, expected_minimum_release_age: u64) -> }) } -fn shell_line_installs_pnpm_with_npm(line: &str) -> bool { - let line = line.split_once('#').map_or(line, |(line, _comment)| line).trim(); - let line = line.strip_prefix("run:").unwrap_or(line).trim(); - let line = line.strip_prefix("-").unwrap_or(line).trim(); - let line = line.trim_matches(|c| c == '"' || c == '\''); - let tokens: Vec<_> = line.split_whitespace().collect(); - - tokens.first() == Some(&"npm") - && tokens.iter().any(|token| *token == "install" || *token == "i") - && tokens.iter().any(|token| { - let token = token.trim_matches(|c: char| c == '"' || c == '\'' || c == ';'); - token == "pnpm" || token.starts_with("pnpm@") - }) -} - fn workflow_installs_pnpm_with_npm(contents: &str) -> bool { - contents.lines().any(shell_line_installs_pnpm_with_npm) + static NPM_INSTALL_PNPM: LazyLock = LazyLock::new(|| Regex::new(r"npm install.*pnpm").unwrap()); + NPM_INSTALL_PNPM.is_match(contents) } fn check_pnpm_release_age_policy() -> Result<()> { @@ -386,7 +374,9 @@ mod tests { #[test] fn detects_direct_npm_pnpm_install() { assert!(workflow_installs_pnpm_with_npm("run: npm install -g pnpm\n")); - assert!(workflow_installs_pnpm_with_npm("run: npm i --global pnpm@10.16.0\n")); + assert!(workflow_installs_pnpm_with_npm( + "run: npm install --global pnpm@10.16.0\n" + )); assert!(workflow_installs_pnpm_with_npm("run: |\n npm install --global pnpm\n")); } From f9755d782fb5ea767c914de91f206ba63b2e9bd5 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Wed, 9 Sep 2026 07:23:58 -0700 Subject: [PATCH 5/5] [bot/widen-pnpm-workflow-lint]: Remove pnpm lint unit tests --- tools/ci/commands/lint/src/main.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tools/ci/commands/lint/src/main.rs b/tools/ci/commands/lint/src/main.rs index c84ca4001e7..fb59d12b502 100644 --- a/tools/ci/commands/lint/src/main.rs +++ b/tools/ci/commands/lint/src/main.rs @@ -366,26 +366,3 @@ fn main() -> Result<()> { Ok(()) } - -#[cfg(test)] -mod tests { - use super::workflow_installs_pnpm_with_npm; - - #[test] - fn detects_direct_npm_pnpm_install() { - assert!(workflow_installs_pnpm_with_npm("run: npm install -g pnpm\n")); - assert!(workflow_installs_pnpm_with_npm( - "run: npm install --global pnpm@10.16.0\n" - )); - assert!(workflow_installs_pnpm_with_npm("run: |\n npm install --global pnpm\n")); - } - - #[test] - fn allows_other_npm_and_pnpm_commands() { - assert!(!workflow_installs_pnpm_with_npm( - "run: npm install --global npm@11.5.1\n" - )); - assert!(!workflow_installs_pnpm_with_npm("run: pnpm install\n")); - assert!(!workflow_installs_pnpm_with_npm("uses: ./.github/actions/setup-pnpm\n")); - } -}