mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(javascript): retain unavailable package metadata during resolution
Consume the same-container inventory lookup before treating a missing package.json projection as absent. Unknown metadata prevents index fallback and unsupported bare-package exports resolution; restored metadata selects its declared main or import/require export. Validation: owner same-container/restored controls and actual ASAR CLI missing/restored package-manifest workflow. Completes the P1 review finding on #3278 that remained after the earlier missing-companion projection fix. Fixes #3213 Related: #3278 Co-authored-by: Geronimo <varungore1@gmail.com>
This commit is contained in:
@@ -272,6 +272,147 @@ describe("unavailable inventoried native add-ons", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("unavailable package metadata", () => {
|
||||
it("blocks directory fallback until same-container manifests are readable", () => {
|
||||
const manifest = {
|
||||
path: "app/relative/package.json",
|
||||
container_sha256: file("app/main.cjs", "root").container_sha256,
|
||||
sha256: null,
|
||||
bytes: 28,
|
||||
inventory_artifact_id: null,
|
||||
inventory_occurrence_id: "missing-relative-manifest",
|
||||
kind: "package-json" as const,
|
||||
hash_status: "unavailable" as const,
|
||||
limitations: ["unpacked manifest companion is missing"],
|
||||
};
|
||||
const files = fileMap([
|
||||
file("app/main.cjs", "root"),
|
||||
file("app/main.mjs", "root"),
|
||||
file("app/relative/index.js", "root"),
|
||||
]);
|
||||
const requestedPaths: string[] = [];
|
||||
const lookup = {
|
||||
get: (path: string) => {
|
||||
requestedPaths.push(path);
|
||||
return path === manifest.path ? manifest : undefined;
|
||||
},
|
||||
hasDirectory: () => false,
|
||||
};
|
||||
const relativeRequire = resolve({
|
||||
declaredPath: "./relative",
|
||||
sourcePath: "app/main.cjs",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files,
|
||||
unanalyzedFiles: lookup,
|
||||
});
|
||||
expect(requestedPaths).toContain(manifest.path);
|
||||
expect(relativeRequire).toMatchObject({
|
||||
resolution_status: "unavailable",
|
||||
resolved_path: null,
|
||||
limitations: ["unpacked manifest companion is missing"],
|
||||
});
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "./relative",
|
||||
sourcePath: "app/main.mjs",
|
||||
context: "module-specifier",
|
||||
moduleKind: "import",
|
||||
files,
|
||||
unanalyzedFiles: lookup,
|
||||
}),
|
||||
).toMatchObject({ resolution_status: "not-found", resolved_path: null });
|
||||
|
||||
const restoredFiles = fileMap([
|
||||
...files.values(),
|
||||
file("app/relative/package.json", "root", '{"main":"public.js"}'),
|
||||
file("app/relative/public.js", "root"),
|
||||
]);
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "./relative",
|
||||
sourcePath: "app/main.cjs",
|
||||
context: "module-specifier",
|
||||
moduleKind: "require",
|
||||
files: restoredFiles,
|
||||
unanalyzedFiles: lookup,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: "app/relative/public.js",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps bare package exports unknown while its manifest is unavailable", () => {
|
||||
const containerSha256 = file("app/main.mjs", "root").container_sha256;
|
||||
const manifest = {
|
||||
path: "app/node_modules/fixture/package.json",
|
||||
container_sha256: containerSha256,
|
||||
sha256: null,
|
||||
bytes: 47,
|
||||
inventory_artifact_id: null,
|
||||
kind: "package-json" as const,
|
||||
hash_status: "unavailable" as const,
|
||||
limitations: ["unpacked exports manifest is missing"],
|
||||
};
|
||||
const files = fileMap([
|
||||
file("app/main.cjs", "root"),
|
||||
file("app/main.mjs", "root"),
|
||||
file("app/node_modules/fixture/index.js", "root"),
|
||||
file("app/node_modules/fixture/import.js", "root"),
|
||||
file("app/node_modules/fixture/require.js", "root"),
|
||||
]);
|
||||
const lookup = {
|
||||
get: (path: string) => (path === manifest.path ? manifest : undefined),
|
||||
hasDirectory: () => false,
|
||||
};
|
||||
for (const [sourcePath, moduleKind] of [
|
||||
["app/main.cjs", "require"],
|
||||
["app/main.mjs", "import"],
|
||||
] as const)
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "fixture",
|
||||
sourcePath,
|
||||
context: "module-specifier",
|
||||
moduleKind,
|
||||
files,
|
||||
unanalyzedFiles: lookup,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "unavailable",
|
||||
resolved_path: null,
|
||||
limitations: ["unpacked exports manifest is missing"],
|
||||
});
|
||||
|
||||
const restoredFiles = fileMap([
|
||||
...files.values(),
|
||||
file(
|
||||
manifest.path,
|
||||
"root",
|
||||
'{"exports":{"import":"./import.js","require":"./require.js"}}',
|
||||
),
|
||||
]);
|
||||
for (const [sourcePath, moduleKind, target] of [
|
||||
["app/main.cjs", "require", "app/node_modules/fixture/require.js"],
|
||||
["app/main.mjs", "import", "app/node_modules/fixture/import.js"],
|
||||
] as const)
|
||||
expect(
|
||||
resolve({
|
||||
declaredPath: "fixture",
|
||||
sourcePath,
|
||||
context: "module-specifier",
|
||||
moduleKind,
|
||||
files: restoredFiles,
|
||||
unanalyzedFiles: lookup,
|
||||
}),
|
||||
).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: target,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("bare package root file lookup", () => {
|
||||
it("uses loader-specific bare package roots when only a sibling .js file exists", () => {
|
||||
const absentDirectory = fileMap([
|
||||
|
||||
@@ -1144,10 +1144,21 @@ const readPackageMetadata = (
|
||||
const path = posix.join(directory, "package.json");
|
||||
const file = input.files.get(path);
|
||||
const source = input.files.get(input.sourcePath);
|
||||
if (
|
||||
file === undefined ||
|
||||
(source !== undefined && file.container_sha256 !== source.container_sha256)
|
||||
)
|
||||
if (file === undefined) {
|
||||
const unavailable = input.unanalyzedFiles?.get(path);
|
||||
if (
|
||||
source !== undefined &&
|
||||
unavailable?.container_sha256 === source.container_sha256
|
||||
)
|
||||
return {
|
||||
kind: "unavailable",
|
||||
limitation:
|
||||
unavailable.limitations?.[0] ??
|
||||
`Directory package metadata ${path} was inventoried but its bytes are unavailable.`,
|
||||
};
|
||||
return { kind: "absent" };
|
||||
}
|
||||
if (source !== undefined && file.container_sha256 !== source.container_sha256)
|
||||
return { kind: "absent" };
|
||||
if (!file.text.included)
|
||||
return {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { rm } from "node:fs/promises";
|
||||
import { createPackageWithOptions } from "@electron/asar";
|
||||
import { rm, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { expect } from "vitest";
|
||||
@@ -15,6 +16,92 @@ import {
|
||||
writeFixtureFiles,
|
||||
} from "../../support/javascriptApplicationFixture.js";
|
||||
|
||||
cliTest(
|
||||
"keeps missing same-container package manifests unavailable until restored",
|
||||
async ({ cli }) => {
|
||||
const root = await createTestTempDirectory(
|
||||
"rea-missing-package-manifests-",
|
||||
);
|
||||
const sourceRoot = join(root, "source");
|
||||
const archive = join(root, "app.asar");
|
||||
const relativeManifest = '{"main":"public.js"}';
|
||||
const exportsManifest =
|
||||
'{"exports":{"import":"./import.js","require":"./require.js"}}';
|
||||
await writeFixtureFiles(sourceRoot, {
|
||||
"main.cjs": 'require("./relative");\nrequire("fixture");\n',
|
||||
"main.mjs": 'import "fixture";\n',
|
||||
"relative/package.json": relativeManifest,
|
||||
"relative/index.js": "export const fallback = true;\n",
|
||||
"relative/public.js": "export const publicEntry = true;\n",
|
||||
"node_modules/fixture/package.json": exportsManifest,
|
||||
"node_modules/fixture/index.js": "export const fallback = true;\n",
|
||||
"node_modules/fixture/import.js": "export const imported = true;\n",
|
||||
"node_modules/fixture/require.js": "module.exports = true;\n",
|
||||
});
|
||||
await createPackageWithOptions(sourceRoot, archive, {
|
||||
unpack: "package.json",
|
||||
});
|
||||
const unpacked = `${archive}.unpacked`;
|
||||
const relativeManifestPath = join(unpacked, "relative/package.json");
|
||||
const exportsManifestPath = join(
|
||||
unpacked,
|
||||
"node_modules/fixture/package.json",
|
||||
);
|
||||
await Promise.all([rm(relativeManifestPath), rm(exportsManifestPath)]);
|
||||
|
||||
const analyze = async () => {
|
||||
const output = await cli.run({
|
||||
arguments: ["analyze-javascript-application", archive, "--json"],
|
||||
});
|
||||
expect(output.exitCode, output.stderr).toBe(0);
|
||||
return javascriptApplicationAnalysisResultSchema.parse(
|
||||
parseEvidence(output.json).normalized_result,
|
||||
);
|
||||
};
|
||||
const unavailable = await analyze();
|
||||
const cjs = findSourceModule(unavailable.graph, "main.cjs");
|
||||
const mjs = findSourceModule(unavailable.graph, "main.mjs");
|
||||
for (const [source, specifier] of [
|
||||
[cjs, "./relative"],
|
||||
[cjs, "fixture"],
|
||||
[mjs, "fixture"],
|
||||
] as const)
|
||||
expect(
|
||||
unavailable.graph.edges.find(
|
||||
(edge) =>
|
||||
edge.source_node_id === source?.node_id &&
|
||||
edge.properties.specifier === specifier,
|
||||
)?.properties,
|
||||
).toMatchObject({
|
||||
resolution_status: "unavailable",
|
||||
resolved_path: null,
|
||||
});
|
||||
expect(unavailable.graph.coverage.status).toBe("partial");
|
||||
|
||||
await Promise.all([
|
||||
writeFile(relativeManifestPath, relativeManifest),
|
||||
writeFile(exportsManifestPath, exportsManifest),
|
||||
]);
|
||||
const restored = await analyze();
|
||||
for (const [sourcePath, specifier, target] of [
|
||||
["main.cjs", "./relative", "relative/public.js"],
|
||||
["main.cjs", "fixture", "node_modules/fixture/require.js"],
|
||||
["main.mjs", "fixture", "node_modules/fixture/import.js"],
|
||||
] as const)
|
||||
expect(
|
||||
restored.graph.edges.find(
|
||||
(edge) =>
|
||||
edge.source_node_id ===
|
||||
findSourceModule(restored.graph, sourcePath)?.node_id &&
|
||||
edge.properties.specifier === specifier,
|
||||
)?.properties,
|
||||
).toMatchObject({
|
||||
resolution_status: "resolved",
|
||||
resolved_path: target,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
cliTest(
|
||||
"resolves ESM specifiers containing whitespace using URL text semantics",
|
||||
async ({ cli }) => {
|
||||
|
||||
Reference in New Issue
Block a user