diff options
| author | David Sherret <dsherret@users.noreply.github.com> | 2024-02-15 14:49:35 -0500 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-02-15 14:49:35 -0500 |
| commit | 4f80d83774ce5402a2b10503529fe422c998b841 (patch) | |
| tree | d99c2e0bdc13e36727c62800130ebcab3b85dae7 /tests | |
| parent | 052b7d8bbdb43eedcdaae1a3094a5f2c70bba279 (diff) | |
feat(unstable): single checksum per JSR package in the lockfile (#22421)
This changes the lockfile to not store JSR specifiers in the "remote"
section. Instead a single JSR integrity is stored per package in the
lockfile, which is a hash of the version's `x.x.x_meta.json` file, which
contains hashes for every file in the package. The hashes in this file
are then compared against when loading.
Additionally, when using `{ "vendor": true }` in a deno.json, the files
can be modified without causing lockfile errors—the checksum is only
checked when copying into the vendor folder and not afterwards
(eventually we should add this behaviour for non-jsr specifiers as
well). As part of this change, the `vendor` folder creation is not
always automatic in the LSP and running an explicit cache command is
necessary. The code required to track checksums in the LSP would have
been too complex for this PR, so that all goes through deno_graph now.
The vendoring is still automatic when running from the CLI.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/integration/jsr_tests.rs | 165 | ||||
| -rw-r--r-- | tests/integration/lsp_tests.rs | 91 | ||||
| -rw-r--r-- | tests/integration/npm_tests.rs | 2 | ||||
| -rw-r--r-- | tests/integration/run_tests.rs | 27 | ||||
| -rw-r--r-- | tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0/mod.ts | 3 | ||||
| -rw-r--r-- | tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0_meta.json | 11 | ||||
| -rw-r--r-- | tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/meta.json | 5 | ||||
| -rw-r--r-- | tests/testdata/npm/lock_file/main.out | 2 |
8 files changed, 263 insertions, 43 deletions
diff --git a/tests/integration/jsr_tests.rs b/tests/integration/jsr_tests.rs index 51cfcfaac..b6e4d8a4f 100644 --- a/tests/integration/jsr_tests.rs +++ b/tests/integration/jsr_tests.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use deno_core::serde_json::json; use deno_core::serde_json::Value; use deno_lockfile::Lockfile; use test_util as util; @@ -182,3 +183,167 @@ fn reload_info_not_found_cache_but_exists_remote() { )) .assert_exit_code(0); } + +#[test] +fn lockfile_bad_package_integrity() { + let test_context = TestContextBuilder::for_jsr().use_temp_cwd().build(); + let temp_dir = test_context.temp_dir(); + + temp_dir.write( + "main.ts", + r#"import version from "jsr:@denotest/no_module_graph@0.1"; + +console.log(version);"#, + ); + temp_dir.write("deno.json", "{}"); // to automatically create a lockfile + + test_context + .new_command() + .args("run --quiet main.ts") + .run() + .assert_matches_text("0.1.1\n"); + + let lockfile_path = temp_dir.path().join("deno.lock"); + let mut lockfile = Lockfile::new(lockfile_path.to_path_buf(), false).unwrap(); + let pkg_name = "@denotest/no_module_graph@0.1.1"; + let original_integrity = get_lockfile_pkg_integrity(&lockfile, pkg_name); + set_lockfile_pkg_integrity(&mut lockfile, pkg_name, "bad_integrity"); + lockfile_path.write(lockfile.as_json_string()); + + let actual_integrity = + test_context.get_jsr_package_integrity("@denotest/no_module_graph/0.1.1"); + let integrity_check_failed_msg = format!("error: Integrity check failed for http://127.0.0.1:4250/@denotest/no_module_graph/0.1.1_meta.json + +Actual: {} +Expected: bad_integrity + at file:///[WILDCARD]/main.ts:1:21 +", actual_integrity); + test_context + .new_command() + .args("run --quiet main.ts") + .run() + .assert_matches_text(&integrity_check_failed_msg) + .assert_exit_code(1); + + // now try with a vendor folder + temp_dir + .path() + .join("deno.json") + .write_json(&json!({ "vendor": true })); + + // should fail again + test_context + .new_command() + .args("run --quiet main.ts") + .run() + .assert_matches_text(&integrity_check_failed_msg) + .assert_exit_code(1); + + // now update to the correct integrity + set_lockfile_pkg_integrity(&mut lockfile, pkg_name, &original_integrity); + lockfile_path.write(lockfile.as_json_string()); + + // should pass now + test_context + .new_command() + .args("run --quiet main.ts") + .run() + .assert_matches_text("0.1.1\n") + .assert_exit_code(0); + + // now update to a bad integrity again + set_lockfile_pkg_integrity(&mut lockfile, pkg_name, "bad_integrity"); + lockfile_path.write(lockfile.as_json_string()); + + // shouldn't matter because we have a vendor folder + test_context + .new_command() + .args("run --quiet main.ts") + .run() + .assert_matches_text("0.1.1\n") + .assert_exit_code(0); + + // now remove the vendor dir and it should fail again + temp_dir.path().join("vendor").remove_dir_all(); + + test_context + .new_command() + .args("run --quiet main.ts") + .run() + .assert_matches_text(&integrity_check_failed_msg) + .assert_exit_code(1); +} + +#[test] +fn bad_manifest_checksum() { + let test_context = TestContextBuilder::for_jsr().use_temp_cwd().build(); + let temp_dir = test_context.temp_dir(); + + temp_dir.write( + "main.ts", + r#"import { add } from "jsr:@denotest/bad-manifest-checksum@1.0.0"; +console.log(add);"#, + ); + + // test it properly checks the checksum on download + test_context + .new_command() + .args("run main.ts") + .run() + .assert_matches_text( + "Download http://127.0.0.1:4250/@denotest/bad-manifest-checksum/meta.json +Download http://127.0.0.1:4250/@denotest/bad-manifest-checksum/1.0.0_meta.json +Download http://127.0.0.1:4250/@denotest/bad-manifest-checksum/1.0.0/mod.ts +error: Integrity check failed. + +Actual: 9a30ac96b5d5c1b67eca69e1e2cf0798817d9578c8d7d904a81a67b983b35cba +Expected: bad-checksum + at file:///[WILDCARD]main.ts:1:21 +", + ) + .assert_exit_code(1); + + // test it properly checks the checksum when loading from the cache + test_context + .new_command() + .args("run main.ts") + .run() + .assert_matches_text( + // ideally the two error messages would be the same... this one comes from + // deno_cache and the one above comes from deno_graph. The thing is, in deno_cache + // (source of this error) it makes sense to include the url in the error message + // because it's not always used in the context of deno_graph + "error: Integrity check failed for http://127.0.0.1:4250/@denotest/bad-manifest-checksum/1.0.0/mod.ts + +Actual: 9a30ac96b5d5c1b67eca69e1e2cf0798817d9578c8d7d904a81a67b983b35cba +Expected: bad-checksum + at file:///[WILDCARD]main.ts:1:21 +", + ) + .assert_exit_code(1); +} + +fn get_lockfile_pkg_integrity(lockfile: &Lockfile, pkg_name: &str) -> String { + lockfile + .content + .packages + .jsr + .get(pkg_name) + .unwrap() + .integrity + .clone() +} + +fn set_lockfile_pkg_integrity( + lockfile: &mut Lockfile, + pkg_name: &str, + integrity: &str, +) { + lockfile + .content + .packages + .jsr + .get_mut(pkg_name) + .unwrap() + .integrity = integrity.to_string(); +} diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 749af95c4..97e9215df 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -693,11 +693,19 @@ fn lsp_format_vendor_path() { .use_http_server() .use_temp_cwd() .build(); + + // put this dependency in the global cache + context + .new_command() + .args("cache http://localhost:4545/run/002_hello.ts") + .run() + .skip_output_check(); + let temp_dir = context.temp_dir(); temp_dir.write("deno.json", json!({ "vendor": true }).to_string()); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.did_open(json!({ + let diagnostics = client.did_open(json!({ "textDocument": { "uri": "file:///a/file.ts", "languageId": "typescript", @@ -705,6 +713,18 @@ fn lsp_format_vendor_path() { "text": r#"import "http://localhost:4545/run/002_hello.ts";"#, }, })); + // copying from the global cache to the local cache requires explicitly + // running the cache command so that the checksums can be verified + assert_eq!( + diagnostics + .all() + .iter() + .map(|d| d.message.as_str()) + .collect::<Vec<_>>(), + vec![ + "Uncached or missing remote URL: http://localhost:4545/run/002_hello.ts" + ] + ); client.write_request( "workspace/executeCommand", json!({ @@ -4358,7 +4378,8 @@ fn lsp_code_actions() { }]) ); let res = client - .write_request( "codeAction/resolve", + .write_request( + "codeAction/resolve", json!({ "title": "Add all missing 'async' modifiers", "kind": "quickfix", @@ -4378,8 +4399,7 @@ fn lsp_code_actions() { "fixId": "fixAwaitInSyncFunction" } }), - ) - ; + ); assert_eq!( res, json!({ @@ -4762,26 +4782,27 @@ fn lsp_code_actions_deno_cache_jsr() { #[test] fn lsp_jsr_lockfile() { - let context = TestContextBuilder::new() - .use_http_server() - .use_temp_cwd() - .build(); + let context = TestContextBuilder::for_jsr().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write("./deno.json", json!({}).to_string()); - temp_dir.write( - "./deno.lock", - json!({ - "version": "3", - "packages": { - "specifiers": { - // This is an old version of the package which exports `sum()` instead - // of `add()`. - "jsr:@denotest/add": "jsr:@denotest/add@0.2.0", - }, - }, - }) - .to_string(), - ); + let lockfile = temp_dir.path().join("deno.lock"); + let integrity = context.get_jsr_package_integrity("@denotest/add/0.2.0"); + lockfile.write_json(&json!({ + "version": "3", + "packages": { + "specifiers": { + // This is an old version of the package which exports `sum()` instead + // of `add()`. + "jsr:@denotest/add": "jsr:@denotest/add@0.2.0", + }, + "jsr": { + "@denotest/add@0.2.0": { + "integrity": integrity + } + } + }, + "remote": {}, + })); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ @@ -4790,8 +4811,8 @@ fn lsp_jsr_lockfile() { "languageId": "typescript", "version": 1, "text": r#" - import { add } from "jsr:@denotest/add"; - console.log(add(1, 2)); + import { sum } from "jsr:@denotest/add"; + console.log(sum(1, 2)); "#, }, })); @@ -10672,9 +10693,27 @@ fn lsp_vendor_dir() { refresh_config(&mut client); let diagnostics = client.read_diagnostics(); - assert_eq!(diagnostics.all().len(), 0, "{:#?}", diagnostics); // cached + // won't be cached until a manual cache occurs + assert_eq!( + diagnostics + .all() + .iter() + .map(|d| d.message.as_str()) + .collect::<Vec<_>>(), + vec![ + "Uncached or missing remote URL: http://localhost:4545/subdir/mod1.ts" + ] + ); + + assert!(!temp_dir + .path() + .join("vendor/http_localhost_4545/subdir/mod1.ts") + .exists()); - // no caching necessary because it was already cached. It should exist now + // now cache + cache(&mut client); + let diagnostics = client.read_diagnostics(); + assert_eq!(diagnostics.all().len(), 0, "{:#?}", diagnostics); // cached assert!(temp_dir .path() .join("vendor/http_localhost_4545/subdir/mod1.ts") diff --git a/tests/integration/npm_tests.rs b/tests/integration/npm_tests.rs index 3777bfe8a..33e331fc3 100644 --- a/tests/integration/npm_tests.rs +++ b/tests/integration/npm_tests.rs @@ -1549,7 +1549,7 @@ fn auto_discover_lock_file() { output .assert_matches_text( r#"Download http://localhost:4545/npm/registry/@denotest/bin -error: Integrity check failed for npm package: "@denotest/bin@1.0.0". Unable to verify that the package +error: Integrity check failed for package: "npm:@denotest/bin@1.0.0". Unable to verify that the package is the same as when the lockfile was generated. Actual: sha512-[WILDCARD] diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 6f108f739..68b72ffed 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -1053,7 +1053,9 @@ fn lock_deno_json_package_json_deps() { "npm:@denotest/esm-basic": "npm:@denotest/esm-basic@1.0.0" }, "jsr": { - "@denotest/module_graph@1.4.0": {} + "@denotest/module_graph@1.4.0": { + "integrity": "555bbe259f55a4a2e7a39e8bf4bcbf25da4c874a313c3e98771eddceedac050b" + } }, "npm": { "@denotest/esm-basic@1.0.0": { @@ -1062,10 +1064,7 @@ fn lock_deno_json_package_json_deps() { } } }, - "remote": { - "http://127.0.0.1:4250/@denotest/module_graph/1.4.0/mod.ts": "5b0ce36e08d759118200d8b4627627b5a89b6261fbb0598e6961a6b287abb699", - "http://127.0.0.1:4250/@denotest/module_graph/1.4.0/other.ts": "9ce27ca439cb0e218b6e1ec26c043dbc0b54c9babc4cb432df478dd1721faade" - }, + "remote": {}, "workspace": { "dependencies": [ "jsr:@denotest/module_graph@1.4", @@ -1106,7 +1105,9 @@ fn lock_deno_json_package_json_deps() { "npm:@denotest/esm-basic": "npm:@denotest/esm-basic@1.0.0" }, "jsr": { - "@denotest/module_graph@1.4.0": {} + "@denotest/module_graph@1.4.0": { + "integrity": "555bbe259f55a4a2e7a39e8bf4bcbf25da4c874a313c3e98771eddceedac050b" + } }, "npm": { "@denotest/esm-basic@1.0.0": { @@ -1115,10 +1116,7 @@ fn lock_deno_json_package_json_deps() { } } }, - "remote": { - "http://127.0.0.1:4250/@denotest/module_graph/1.4.0/mod.ts": "5b0ce36e08d759118200d8b4627627b5a89b6261fbb0598e6961a6b287abb699", - "http://127.0.0.1:4250/@denotest/module_graph/1.4.0/other.ts": "9ce27ca439cb0e218b6e1ec26c043dbc0b54c9babc4cb432df478dd1721faade" - }, + "remote": {}, "workspace": { "dependencies": [ "jsr:@denotest/module_graph@1.4" @@ -1147,13 +1145,12 @@ fn lock_deno_json_package_json_deps() { "jsr:@denotest/module_graph@1.4": "jsr:@denotest/module_graph@1.4.0", }, "jsr": { - "@denotest/module_graph@1.4.0": {} + "@denotest/module_graph@1.4.0": { + "integrity": "555bbe259f55a4a2e7a39e8bf4bcbf25da4c874a313c3e98771eddceedac050b" + } } }, - "remote": { - "http://127.0.0.1:4250/@denotest/module_graph/1.4.0/mod.ts": "5b0ce36e08d759118200d8b4627627b5a89b6261fbb0598e6961a6b287abb699", - "http://127.0.0.1:4250/@denotest/module_graph/1.4.0/other.ts": "9ce27ca439cb0e218b6e1ec26c043dbc0b54c9babc4cb432df478dd1721faade" - }, + "remote": {}, "workspace": { "dependencies": [ "jsr:@denotest/module_graph@1.4" diff --git a/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0/mod.ts b/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0/mod.ts new file mode 100644 index 000000000..8d9b8a22a --- /dev/null +++ b/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0/mod.ts @@ -0,0 +1,3 @@ +export function add(a: number, b: number): number { + return a + b; +} diff --git a/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0_meta.json b/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0_meta.json new file mode 100644 index 000000000..8ef8ab3c3 --- /dev/null +++ b/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/1.0.0_meta.json @@ -0,0 +1,11 @@ +{ + "exports": { + ".": "./mod.ts" + }, + "manifest": { + "/mod.ts": { + "size": 0, + "checksum": "sha256-bad-checksum" + } + } +} diff --git a/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/meta.json b/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/meta.json new file mode 100644 index 000000000..02601e4d0 --- /dev/null +++ b/tests/testdata/jsr/registry/@denotest/bad-manifest-checksum/meta.json @@ -0,0 +1,5 @@ +{ + "versions": { + "1.0.0": {} + } +} diff --git a/tests/testdata/npm/lock_file/main.out b/tests/testdata/npm/lock_file/main.out index 65e881be6..dead1a623 100644 --- a/tests/testdata/npm/lock_file/main.out +++ b/tests/testdata/npm/lock_file/main.out @@ -1,5 +1,5 @@ Download [WILDCARD] -error: Integrity check failed for npm package: "@babel/parser@7.19.0". Unable to verify that the package +error: Integrity check failed for package: "npm:@babel/parser@7.19.0". Unable to verify that the package is the same as when the lockfile was generated. Actual: sha512-74bEXKX2h+8rrfQUfsBfuZZHzsEs6Eql4pqy/T4Nn6Y9wNPggQOqD6z6pn5Bl8ZfysKouFZT/UXEH94ummEeQw== |
