openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-28 18:58:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: parse npm lock graph directly

Browse Source
This commit is contained in:
Casey Silver
2026-07-07 22:19:13 -07:00
parent 48cdaf27d1
commit dbed34103c
5 changed files with 126 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/tools/handlers/dependency_check.rs
+17 -31
View File
@@ -35,7 +35,6 @@ use codex_dependency_check::detect_dependency_install_command;
use codex_dependency_check::npm_ci_command;
use codex_dependency_check::npm_install_command;
use codex_dependency_check::npm_query_installed_command;
use codex_dependency_check::npm_query_lock_command;
use codex_dependency_check::npm_rebuild_command;
use codex_dependency_check::validate_npm_manifest;
use codex_protocol::exec_output::ExecToolCallOutput;
@@ -172,13 +171,7 @@ impl DependencyCheckHandler {
&resolve,
));
}
let checked_graph = match query_graph(
&runner,
resolution_dir.path(),
WorkingDirectoryAccess::PreapprovedScratch,
)
.await?
{
let checked_graph = match read_lock_graph(resolution_dir.path()).await {
Ok(graph) => graph,
Err(message) => return Ok(blocked_output(message)),
};
@@ -214,11 +207,10 @@ impl DependencyCheckHandler {
if lock_update.exit_code != 0 {
return Ok(command_failure_output("project lock update", &lock_update));
}
let locked_graph =
match query_graph(&runner, &workdir, WorkingDirectoryAccess::Default).await? {
Ok(graph) => graph,
Err(message) => return Ok(blocked_output(message)),
};
let locked_graph = match read_lock_graph(&workdir).await {
Ok(graph) => graph,
Err(message) => return Ok(blocked_output(message)),
};
if let Err(mismatch) = checked_graph.compare(&locked_graph) {
return Ok(graph_mismatch_output("project lock update", &mismatch));
}
@@ -278,25 +270,19 @@ pub(crate) fn dependency_manifest_edit_message() -> String {
.to_string()
}
async fn query_graph(
runner: &DependencyCommandRunner,
cwd: &Path,
working_directory_access: WorkingDirectoryAccess,
) -> Result<Result<NpmGraph, String>, FunctionCallError> {
let query = runner
.run(
npm_query_lock_command(),
cwd,
ScriptPolicy::Disabled,
working_directory_access,
"Read the exact npm lock graph for dependency verification.",
async fn read_lock_graph(cwd: &Path) -> Result<NpmGraph, String> {
let lockfile_path = cwd.join("package-lock.json");
let lockfile = read_regular_file(&lockfile_path).await.map_err(|err| {
format!(
"Dependency Check stopped before lifecycle scripts because it could not read {}: {err}",
lockfile_path.display()
)
.await?;
if query.exit_code != 0 {
return Ok(Err(command_failure_message("npm graph query", &query)));
}
Ok(NpmGraph::from_query_json(&query.stdout.text)
.map_err(|err| format!("Dependency Check stopped before lifecycle scripts because npm returned an unverifiable graph: {err}")))
})?;
NpmGraph::from_package_lock_json(&lockfile).map_err(|err| {
format!(
"Dependency Check stopped before lifecycle scripts because npm produced an unverifiable package-lock.json graph: {err}"
)
})
}
async fn query_installed_graph(
codex-rs/dependency-check/src/lib.rs
-1
View File
@@ -17,7 +17,6 @@ pub use npm::NpmManifestError;
pub use npm::npm_ci_command;
pub use npm::npm_install_command;
pub use npm::npm_query_installed_command;
pub use npm::npm_query_lock_command;
pub use npm::npm_rebuild_command;
pub use npm::validate_npm_manifest;
pub use osv::DependencyPolicyAction;
codex-rs/dependency-check/src/npm.rs
+105 -32
View File
@@ -1,6 +1,7 @@
use crate::DependencyCheckRequest;
use serde::Deserialize;
use serde_json::Value;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use thiserror::Error;
use url::Url;
@@ -31,18 +32,28 @@ pub struct NpmInstalledGraph {
}
impl NpmGraph {
pub fn from_query_json(json: &str) -> Result<Self, NpmGraphError> {
let nodes: Vec<NpmQueryNode> = serde_json::from_str(json)?;
pub fn from_package_lock_json(json: &str) -> Result<Self, NpmGraphError> {
let lockfile: NpmPackageLock = serde_json::from_str(json)?;
if !matches!(lockfile.lockfile_version, 2 | 3) {
return Err(NpmGraphError::UnsupportedLockfileVersion(
lockfile.lockfile_version,
));
}
let mut packages = BTreeSet::new();
for node in nodes {
if node.location.as_deref() == Some("") {
for (location, package) in lockfile.packages {
if location.is_empty() {
continue;
}
let name = node.name.ok_or(NpmGraphError::MissingField("name"))?;
let version = node.version.ok_or(NpmGraphError::MissingField("version"))?;
let resolved = node
let name = package
.name
.or_else(|| package_name_from_lockfile_location(&location))
.ok_or(NpmGraphError::MissingField("name"))?;
let version = package
.version
.ok_or(NpmGraphError::MissingField("version"))?;
let resolved = package
.resolved
.ok_or_else(|| NpmGraphError::UnsupportedSource {
name: name.clone(),
@@ -61,7 +72,7 @@ impl NpmGraph {
resolved: Some(resolved),
});
}
let integrity = node
let integrity = package
.integrity
.filter(|integrity| !integrity.is_empty())
.ok_or_else(|| NpmGraphError::MissingIntegrity {
@@ -202,10 +213,12 @@ impl std::error::Error for NpmInstalledGraphMismatch {}
#[derive(Debug, Error)]
pub enum NpmGraphError {
#[error("npm query returned invalid JSON: {0}")]
#[error("npm graph input is invalid JSON: {0}")]
InvalidJson(#[from] serde_json::Error),
#[error("npm query result is missing required field `{0}`")]
#[error("npm graph entry is missing required field `{0}`")]
MissingField(&'static str),
#[error("npm package-lock version {0} is unsupported; expected version 2 or 3")]
UnsupportedLockfileVersion(u64),
#[error("npm package `{name}@{version}` has an unsupported resolved source: {resolved:?}")]
UnsupportedSource {
name: String,
@@ -278,16 +291,6 @@ pub fn npm_install_command(
command
}
pub fn npm_query_lock_command() -> Vec<String> {
vec![
"npm".to_string(),
"query".to_string(),
"*".to_string(),
"--json".to_string(),
"--package-lock-only".to_string(),
]
}
pub fn npm_ci_command() -> Vec<String> {
vec![
"npm".to_string(),
@@ -316,10 +319,45 @@ struct NpmQueryNode {
name: Option<String>,
version: Option<String>,
resolved: Option<String>,
integrity: Option<String>,
location: Option<String>,
}
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
struct NpmPackageLock {
lockfile_version: u64,
#[serde(default)]
packages: BTreeMap<String, NpmLockPackage>,
}
#[derive(Debug, Deserialize)]
struct NpmLockPackage {
name: Option<String>,
version: Option<String>,
resolved: Option<String>,
integrity: Option<String>,
}
fn package_name_from_lockfile_location(location: &str) -> Option<String> {
let (_, package_path) = location.rsplit_once("node_modules/")?;
let mut components = package_path.split('/');
let first = components.next()?;
if first.is_empty() {
return None;
}
if first.starts_with('@') {
let second = components.next()?;
if second.is_empty() || components.next().is_some() {
return None;
}
Some(format!("{first}/{second}"))
} else if components.next().is_none() {
Some(first.to_string())
} else {
None
}
}
struct ParsedNpmQueryNode {
name: String,
version: String,
@@ -405,11 +443,14 @@ mod tests {
#[test]
fn parses_and_compares_lock_graphs() {
let graph = NpmGraph::from_query_json(
r#"[
{"name":"example","version":"1.0.0","location":"","resolved":null},
{"name":"zod","version":"3.23.8","location":"node_modules/zod","resolved":"https://registry.npmjs.org/zod/-/zod-3.23.8.tgz","integrity":"sha512-example"}
]"#,
let graph = NpmGraph::from_package_lock_json(
r#"{
"lockfileVersion": 3,
"packages": {
"": {"name":"example","version":"1.0.0"},
"node_modules/zod": {"version":"3.23.8","resolved":"https://registry.npmjs.org/zod/-/zod-3.23.8.tgz","integrity":"sha512-example"}
}
}"#,
)
.expect("parse graph");
assert_eq!(
@@ -430,12 +471,12 @@ mod tests {
#[test]
fn reports_lock_and_installed_graph_mismatches() {
let checked = NpmGraph::from_query_json(
r#"[{"name":"zod","version":"3.23.8","location":"node_modules/zod","resolved":"https://registry.npmjs.org/zod/-/zod-3.23.8.tgz","integrity":"sha512-checked"}]"#,
let checked = NpmGraph::from_package_lock_json(
r#"{"lockfileVersion":3,"packages":{"node_modules/zod":{"version":"3.23.8","resolved":"https://registry.npmjs.org/zod/-/zod-3.23.8.tgz","integrity":"sha512-checked"}}}"#,
)
.expect("checked graph");
let changed = NpmGraph::from_query_json(
r#"[{"name":"zod","version":"3.24.0","location":"node_modules/zod","resolved":"https://registry.npmjs.org/zod/-/zod-3.24.0.tgz","integrity":"sha512-changed"}]"#,
let changed = NpmGraph::from_package_lock_json(
r#"{"lockfileVersion":3,"packages":{"node_modules/zod":{"version":"3.24.0","resolved":"https://registry.npmjs.org/zod/-/zod-3.24.0.tgz","integrity":"sha512-changed"}}}"#,
)
.expect("changed graph");
let installed = NpmInstalledGraph::from_query_json(
@@ -449,13 +490,45 @@ mod tests {
#[test]
fn rejects_unverifiable_sources() {
let err = NpmGraph::from_query_json(
r#"[{"name":"local","version":"1.0.0","location":"node_modules/local","resolved":"file:../local"}]"#,
let err = NpmGraph::from_package_lock_json(
r#"{"lockfileVersion":3,"packages":{"node_modules/local":{"version":"1.0.0","resolved":"file:../local"}}}"#,
)
.expect_err("local source should fail");
assert!(matches!(err, NpmGraphError::UnsupportedSource { .. }));
}
#[test]
fn derives_scoped_and_nested_package_names_from_lockfile_locations() {
let graph = NpmGraph::from_package_lock_json(
r#"{
"lockfileVersion": 2,
"packages": {
"node_modules/@scope/top": {"version":"1.0.0","resolved":"https://registry.npmjs.org/@scope/top/-/top-1.0.0.tgz","integrity":"sha512-top"},
"node_modules/parent/node_modules/@scope/nested": {"version":"2.0.0","resolved":"https://registry.npmjs.org/@scope/nested/-/nested-2.0.0.tgz","integrity":"sha512-nested"}
}
}"#,
)
.expect("parse graph");
assert_eq!(
graph.coordinates(),
BTreeSet::from([
("@scope/nested".to_string(), "2.0.0".to_string()),
("@scope/top".to_string(), "1.0.0".to_string()),
])
);
}
#[test]
fn rejects_legacy_lockfile_without_packages_graph() {
let err = NpmGraph::from_package_lock_json(
r#"{"lockfileVersion":1,"dependencies":{"zod":{"version":"3.23.8"}}}"#,
)
.expect_err("legacy lockfile should fail");
assert!(matches!(err, NpmGraphError::UnsupportedLockfileVersion(1)));
}
#[test]
fn rejects_workspaces_and_non_npm_projects() {
assert!(matches!(
codex-rs/dependency-check/src/osv.rs
+2 -2
View File
@@ -198,8 +198,8 @@ mod tests {
use wiremock::matchers::path;
fn graph() -> NpmGraph {
NpmGraph::from_query_json(
r#"[{"name":"example","version":"1.0.0","location":"node_modules/example","resolved":"https://registry.npmjs.org/example/-/example-1.0.0.tgz","integrity":"sha512-example"}]"#,
NpmGraph::from_package_lock_json(
r#"{"lockfileVersion":3,"packages":{"node_modules/example":{"version":"1.0.0","resolved":"https://registry.npmjs.org/example/-/example-1.0.0.tgz","integrity":"sha512-example"}}}"#,
)
.expect("parse graph")
}