Skip to content

fix(npm,lint): remove hardcoded allowlists, forward args untouched #2860

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

Open
guyoron1 wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix(npm,lint): remove hardcoded allowlists, forward args untouched #2860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54542e9
fix(npm,lint): remove hardcoded allowlists, forward args untouched
guyoron1 Jul 7, 2026
51e383f
fix(pnpm): add generic script routing so bare pnpm <script> gets filt…
guyoron1 Jul 13, 2026
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
128 changes: 2 additions & 126 deletions src/cmds/js/npm_cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,97 +1,11 @@
//! Filters npm output and auto-injects the "run" subcommand when appropriate.
//! Filters npm output — strips boilerplate, progress bars, and warnings.

use crate::core::runner;
use crate::core::utils::resolved_command;
use anyhow::Result;

/// Known npm subcommands that should NOT get "run" injected.
/// Shared between production code and tests to avoid drift.
const NPM_SUBCOMMANDS: &[&str] = &[
"install",
"i",
"ci",
"uninstall",
"remove",
"rm",
"update",
"up",
"list",
"ls",
"outdated",
"init",
"create",
"publish",
"pack",
"link",
"audit",
"fund",
"exec",
"explain",
"why",
"search",
"view",
"info",
"show",
"config",
"set",
"get",
"cache",
"prune",
"dedupe",
"doctor",
"help",
"version",
"prefix",
"root",
"bin",
"bugs",
"docs",
"home",
"repo",
"ping",
"whoami",
"token",
"profile",
"team",
"access",
"owner",
"deprecate",
"dist-tag",
"star",
"stars",
"login",
"logout",
"adduser",
"unpublish",
"pkg",
"diff",
"rebuild",
"test",
"t",
"start",
"stop",
"restart",
];

pub fn run(args: &[String], verbose: u8, skip_env: bool) -> Result<i32> {
// Determine if this is "npm run <script>" or another npm subcommand (install, list, etc.)
// Only inject "run" when args look like a script name, not a known npm subcommand.
let first_arg = args.first().map(|s| s.as_str());
let is_run_explicit = first_arg == Some("run");
let is_npm_subcommand = first_arg
.map(|a| NPM_SUBCOMMANDS.contains(&a) || a.starts_with('-'))
.unwrap_or(false);

let mut effective_args: Vec<String> = Vec::with_capacity(args.len() + 1);
if is_run_explicit || is_npm_subcommand {
effective_args.extend_from_slice(args);
} else {
// "rtk npm build" → "npm run build" (assume script name)
effective_args.push("run".to_string());
effective_args.extend_from_slice(args);
}

run_filtered("npm", &effective_args, verbose, skip_env)
run_filtered("npm", args, verbose, skip_env)
}

/// Run an npx tool through the same filtered pipeline as `npm`.
Expand Down Expand Up @@ -190,44 +104,6 @@ npm notice
assert!(result.contains("Build completed"));
}

#[test]
fn test_npm_subcommand_routing() {
// Uses the shared NPM_SUBCOMMANDS constant — no drift between prod and test
fn needs_run_injection(args: &[&str]) -> bool {
let first = args.first().copied();
let is_run_explicit = first == Some("run");
let is_subcommand = first
.map(|a| NPM_SUBCOMMANDS.contains(&a) || a.starts_with('-'))
.unwrap_or(false);
!is_run_explicit && !is_subcommand
}

// Known subcommands should NOT get "run" injected
for subcmd in NPM_SUBCOMMANDS {
assert!(
!needs_run_injection(&[subcmd]),
"'npm {}' should NOT inject 'run'",
subcmd
);
}

// Script names SHOULD get "run" injected
for script in &["build", "dev", "lint", "typecheck", "deploy"] {
assert!(
needs_run_injection(&[script]),
"'npm {}' SHOULD inject 'run'",
script
);
}

// Flags should NOT get "run" injected
assert!(!needs_run_injection(&["--version"]));
assert!(!needs_run_injection(&["-h"]));

// Explicit "run" should NOT inject another "run"
assert!(!needs_run_injection(&["run", "build"]));
}

#[test]
fn test_filter_npm_output_empty() {
let output = "\n\n\n";
Expand Down
130 changes: 108 additions & 22 deletions src/discover/registry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3837,16 +3837,12 @@ mod tests {
"npm exec eslint",
"npm rum biome",
"npm rum eslint",
"npm rum lint",
"npm run biome",
"npm run eslint",
"npm run lint",
"npm run-script biome",
"npm run-script eslint",
"npm run-script lint",
"npm urn biome",
"npm urn eslint",
"npm urn lint",
"npm x biome",
"npm x eslint",
"pnpm dlx biome",
Expand All @@ -3855,25 +3851,18 @@ mod tests {
"pnpm exec eslint",
"pnpm run biome",
"pnpm run eslint",
"pnpm run lint",
"pnpm run-script biome",
"pnpm run-script eslint",
"pnpm run-script lint",
"npm biome",
"npm eslint",
"npm lint",
"npx biome",
"npx eslint",
"npx lint",
"pnpm biome",
"pnpm eslint",
"pnpm lint",
"pnpx biome",
"pnpx eslint",
"pnpx lint",
"biome",
"eslint",
"lint",
];
for command in commands {
assert!(
Expand All @@ -3890,23 +3879,49 @@ mod tests {
}
}

#[test]
fn test_classify_lint_scripts_not_rewritten() {
let script_commands = vec![
"lint",
"npm lint",
"npm run lint",
"npm rum lint",
"npm urn lint",
"npm run-script lint",
"npx lint",
"pnpm lint",
"pnpm run lint",
"pnpm run-script lint",
"pnpx lint",
];
for command in script_commands {
assert!(
!matches!(
classify_command(command),
Classification::Supported {
rtk_equivalent: "rtk lint",
..
}
),
"'{}' should NOT classify as rtk lint (it's a script, not a linter binary)",
command
);
}
}

#[test]
fn test_rewrite_lint() {
let commands = vec![
"npm exec biome",
"npm exec eslint",
"npm rum biome",
"npm rum eslint",
"npm rum lint",
"npm run biome",
"npm run eslint",
"npm run lint",
"npm run-script biome",
"npm run-script eslint",
"npm run-script lint",
"npm urn biome",
"npm urn eslint",
"npm urn lint",
"npm x biome",
"npm x eslint",
"pnpm dlx biome",
Expand All @@ -3915,25 +3930,18 @@ mod tests {
"pnpm exec eslint",
"pnpm run biome",
"pnpm run eslint",
"pnpm run lint",
"pnpm run-script biome",
"pnpm run-script eslint",
"pnpm run-script lint",
"npm biome",
"npm eslint",
"npm lint",
"npx biome",
"npx eslint",
"npx lint",
"pnpm biome",
"pnpm eslint",
"pnpm lint",
"pnpx biome",
"pnpx eslint",
"pnpx lint",
"biome",
"eslint",
"lint",
];
for command in commands {
assert_eq!(
Expand All @@ -3945,6 +3953,84 @@ mod tests {
}
}

#[test]
fn test_rewrite_lint_scripts_not_rewritten_to_rtk_lint() {
let script_commands = vec![
"lint",
"npm lint",
"npm run lint",
"pnpm lint",
"pnpm run lint",
"npx lint",
"pnpx lint",
];
for command in script_commands {
let result = rewrite_command_no_prefixes(command, &[]);
assert_ne!(
result,
Some("rtk lint".into()),
"'{}' should NOT be rewritten to rtk lint (got {:?})",
command,
result
);
}
}

#[test]
fn test_pnpm_scripts_route_through_rtk_pnpm() {
let script_commands = vec![
("pnpm lint", "rtk pnpm lint"),
("pnpm build", "rtk pnpm build"),
("pnpm dev", "rtk pnpm dev"),
("pnpm test", "rtk pnpm test"),
("pnpm typecheck", "rtk pnpm typecheck"),
("pnpm run lint", "rtk pnpm run lint"),
("pnpm run build", "rtk pnpm run build"),
];
for (command, expected) in script_commands {
assert_eq!(
classify_command(command),
Classification::Supported {
rtk_equivalent: "rtk pnpm",
category: "PackageManager",
estimated_savings_pct: 80.0,
status: RtkStatus::Existing,
},
"'{}' should classify as rtk pnpm",
command
);
assert_eq!(
rewrite_command_no_prefixes(command, &[]),
Some(expected.into()),
"'{}' should rewrite to '{}'",
command,
expected
);
}
}

#[test]
fn test_pnpm_specific_tools_still_override_generic() {
let specific_commands = vec![
("pnpm eslint", "rtk lint"),
("pnpm biome", "rtk lint"),
("pnpm vitest", "rtk vitest"),
("pnpm jest", "rtk jest"),
("pnpm tsc", "rtk tsc"),
("pnpm prettier", "rtk prettier"),
("pnpm playwright", "rtk playwright"),
];
for (command, expected_rtk) in specific_commands {
assert_eq!(
rewrite_command_no_prefixes(command, &[]),
Some(expected_rtk.into()),
"'{}' should route to specific '{}', not generic rtk pnpm",
command,
expected_rtk
);
}
}

#[test]
fn test_classify_jest() {
let commands = vec![
Expand Down
Loading
Loading