diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 18a8081..b8f1218 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -21,6 +21,7 @@ import { } from "@git-agentic/sentinel-core"; import { createSandbox, runLifecycleScripts, scrubEnv } from "@git-agentic/sentinel-sandbox"; import { formatReport, formatManifest, verdictExitCode, formatTree, treeExitCode, formatViolations, formatStats, formatHistory, formatExplain, formatLint, formatPreview, type Manifest, type ViolationRow, type ExplainResult, type PreviewResult } from "./format.js"; +import { packageCoordinateFromTarball } from "./scan-coordinate.js"; const DEFAULT_PROXY = process.env.SENTINEL_PROXY ?? "http://localhost:4873"; @@ -55,10 +56,10 @@ program .option("--json", "emit the raw JSON report", false) .action(async (tarballPath: string, opts: { json: boolean }) => { const tarball = readFileSync(tarballPath) as Buffer; - const name = tarballPath.split("/").pop()?.replace(/\.tgz$/, "") ?? "local"; + const { name, version } = await packageCoordinateFromTarball(tarball); const report = await auditTarball({ meta: { - name, version: "local", author: null, maintainers: [], license: null, + name, version, author: null, maintainers: [], license: null, hasInstallScripts: false, }, tarball, diff --git a/packages/cli/src/scan-coordinate.ts b/packages/cli/src/scan-coordinate.ts new file mode 100644 index 0000000..4b0eae4 --- /dev/null +++ b/packages/cli/src/scan-coordinate.ts @@ -0,0 +1,24 @@ +import { Buffer } from "node:buffer"; +import { extractTarball, readPackageManifest, type PackageCoordinate } from "@git-agentic/sentinel-core"; + +export type { PackageCoordinate }; + +/** + * The package coordinate as the tarball itself states it. + * + * COD2-1058: `scan` used to derive this from the tarball's filename, which + * `npm pack` writes as `-.tgz` — so the "name" carried the + * version and never matched a real package name. That value fed typosquat + * detection, policy.deny, waiver matching and the exit code. + * + * Extraction and manifest reading both already exist in + * `@git-agentic/sentinel-core` for `runAudit`'s own publish-time identity + * check — this calls the same `extractTarball` + `readPackageManifest` + * rather than re-deriving either, so the two callers can't drift again. + */ +export async function packageCoordinateFromTarball( + tarball: Buffer, +): Promise { + const extracted = await extractTarball(tarball); + return readPackageManifest(extracted); +} diff --git a/packages/cli/test/helpers/make-tarball.ts b/packages/cli/test/helpers/make-tarball.ts new file mode 100644 index 0000000..5790f9e --- /dev/null +++ b/packages/cli/test/helpers/make-tarball.ts @@ -0,0 +1,47 @@ +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, dirname } from "node:path"; +import { create } from "tar"; + +export interface MakeTarballOptions { + /** Package name to write into package/package.json. Ignored when omitManifest is set. */ + name?: string; + /** Package version to write into package/package.json. Ignored when omitManifest is set. */ + version?: string; + /** + * Build a tarball with no package/package.json at all — the "no readable + * manifest" case the coordinate reader must refuse rather than guess past. + */ + omitManifest?: boolean; +} + +/** + * Build a real in-memory npm-style .tgz (gzip+tar, `package/` prefix), the + * same shape `npm pack` produces. Used to test that the scan coordinate is + * read from the manifest inside the tarball, not from its filename. + */ +export async function makeTarball(opts: MakeTarballOptions = {}): Promise { + const dir = mkdtempSync(join(tmpdir(), "sentinel-make-tarball-")); + try { + const files: Record = opts.omitManifest + ? { "package/README.md": "no manifest here\n" } + : { + "package/package.json": JSON.stringify( + { name: opts.name ?? "example", version: opts.version ?? "0.0.0" }, + null, + 2, + ), + }; + for (const [path, contents] of Object.entries(files)) { + const full = join(dir, path); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, contents); + } + const chunks: Buffer[] = []; + const stream = create({ cwd: dir, gzip: true }, Object.keys(files)); + for await (const chunk of stream) chunks.push(Buffer.from(chunk)); + return Buffer.concat(chunks); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} diff --git a/packages/cli/test/scan.test.ts b/packages/cli/test/scan.test.ts new file mode 100644 index 0000000..052a5aa --- /dev/null +++ b/packages/cli/test/scan.test.ts @@ -0,0 +1,36 @@ +import assert from "node:assert/strict"; +import { describe, test } from "node:test"; +import { readFileSync, writeFileSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { packageCoordinateFromTarball } from "../src/scan-coordinate.js"; +import { makeTarball } from "./helpers/make-tarball.js"; + +describe("sentinel scan coordinate", () => { + test("reads the name and version from the manifest, not the filename", async () => { + // npm pack names tarballs -.tgz, so a filename-derived + // name is version-suffixed and never equals the real package name. + const tarball = await makeTarball({ name: "lodash", version: "4.17.21" }); + const dir = mkdtempSync(join(tmpdir(), "scan-")); + try { + const path = join(dir, "lodash-4.17.21.tgz"); + writeFileSync(path, tarball); + + const coordinate = await packageCoordinateFromTarball(readFileSync(path)); + + assert.equal(coordinate.name, "lodash"); + assert.equal(coordinate.version, "4.17.21"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("refuses a tarball with no readable manifest rather than guessing", async () => { + const tarball = await makeTarball({ omitManifest: true }); + + await assert.rejects( + () => packageCoordinateFromTarball(tarball), + /missing readable package\/package\.json/, + ); + }); +}); diff --git a/packages/core/src/audit.ts b/packages/core/src/audit.ts index fc153d5..7c9df87 100644 --- a/packages/core/src/audit.ts +++ b/packages/core/src/audit.ts @@ -1,5 +1,6 @@ import { Buffer } from "node:buffer"; import { baselineFrom, extractTarball, integrityOf } from "./extract.js"; +import { readPackageManifest } from "./manifest.js"; import { extractCapabilities, diffCapabilities } from "./capabilities.js"; import { RULES } from "./rules/index.js"; import { capabilityNoveltyFindings } from "./rules/capability-novelty.js"; @@ -118,17 +119,7 @@ export async function runAudit(input: AuditTarballInput): Promise { // not replace that complete report with a secondary "missing manifest" parse // error caused by the intentionally-truncated extraction. if (input.requirePackageManifest && !extracted.truncated) { - if (extracted.packageManifestEntryCount > 1) { - throw new Error("malformed npm tarball: duplicate package/package.json entries"); - } - const file = extracted.files.find((f) => f.path === "package/package.json"); - if (!file) throw new Error("malformed npm tarball: missing readable package/package.json"); - let manifest: { name?: unknown; version?: unknown }; - try { manifest = JSON.parse(file.content) as { name?: unknown; version?: unknown }; } - catch { throw new Error("malformed npm tarball: package/package.json is not valid JSON"); } - if (typeof manifest.name !== "string" || typeof manifest.version !== "string") { - throw new Error("malformed npm tarball: package/package.json requires string name and version"); - } + const manifest = readPackageManifest(extracted); if (manifest.name !== input.requirePackageManifest.name || manifest.version !== input.requirePackageManifest.version) { throw new Error( `malformed npm tarball: package/package.json identity ${manifest.name}@${manifest.version} does not match publish target ${input.requirePackageManifest.name}@${input.requirePackageManifest.version}`, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3937426..96d98e3 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -36,6 +36,7 @@ export { type SriAlgorithm, type ExtractResult, } from "./extract.js"; +export { readPackageManifest, type PackageCoordinate } from "./manifest.js"; export { RULES } from "./rules/index.js"; export { SENSITIVE_PATHS, sensitivePathsFor, type SensitivePath } from "./sensitive-paths.js"; export { extractCapabilities, diffCapabilities } from "./capabilities.js"; diff --git a/packages/core/src/manifest.ts b/packages/core/src/manifest.ts new file mode 100644 index 0000000..d4e084e --- /dev/null +++ b/packages/core/src/manifest.ts @@ -0,0 +1,45 @@ +import type { ExtractResult } from "./extract.js"; + +export interface PackageCoordinate { + name: string; + version: string; +} + +/** + * Locate, parse, and minimally validate `package/package.json` from an + * already-extracted tarball, returning the name/version it declares. + * + * COD2-1058: this used to be written twice — once inline in `runAudit`'s + * `requirePackageManifest` branch (the publish-gate identity check) and once + * in the CLI's `scan` coordinate reader — and the two copies had already + * drifted on error wording. This is the one place that owns "how do we read + * the coordinate a tarball itself declares." A caller that additionally needs + * to check that coordinate against something else (a known publish target, a + * filename, …) does that check on top of this result; it does not re-derive + * name/version itself. + * + * Refuses rather than falls back on any ambiguity — a manifest that cannot be + * read unambiguously is a coordinate that is unknown, not one to guess at. + */ +export function readPackageManifest(extracted: ExtractResult): PackageCoordinate { + if (extracted.packageManifestEntryCount > 1) { + throw new Error("malformed npm tarball: duplicate package/package.json entries"); + } + const file = extracted.files.find((f) => f.path === "package/package.json"); + if (!file) { + throw new Error("malformed npm tarball: missing readable package/package.json"); + } + let manifest: { name?: unknown; version?: unknown }; + try { + manifest = JSON.parse(file.content) as { name?: unknown; version?: unknown }; + } catch { + throw new Error("malformed npm tarball: package/package.json is not valid JSON"); + } + if (typeof manifest.name !== "string" || manifest.name.length === 0) { + throw new Error("malformed npm tarball: package/package.json declares no name"); + } + if (typeof manifest.version !== "string" || manifest.version.length === 0) { + throw new Error("malformed npm tarball: package/package.json declares no version"); + } + return { name: manifest.name, version: manifest.version }; +} diff --git a/packages/core/test/manifest.test.ts b/packages/core/test/manifest.test.ts new file mode 100644 index 0000000..0949623 --- /dev/null +++ b/packages/core/test/manifest.test.ts @@ -0,0 +1,85 @@ +import assert from "node:assert/strict"; +import { describe, test } from "node:test"; +import { readPackageManifest } from "../src/manifest.js"; +import type { ExtractResult } from "../src/extract.js"; +import type { PackageFile } from "../src/types.js"; + +/** + * Build a minimal, well-formed ExtractResult around a set of files — + * readPackageManifest only reads `files` and `packageManifestEntryCount`, so + * every other field is a fixed, unexercised default. + */ +function fakeExtracted(files: PackageFile[], packageManifestEntryCount: number): ExtractResult { + return { + files, + packageManifestEntryCount, + unpackedSize: 0, + fileCount: files.length, + truncated: false, + unscanned: [], + unscannedTotals: { count: 0, native: 0, bytes: 0 }, + contentMismatch: [], + contentMismatchTotals: { count: 0, byKind: {} }, + }; +} + +function manifestFile(content: string): PackageFile { + return { path: "package/package.json", content, size: content.length, changed: true }; +} + +describe("readPackageManifest", () => { + test("reads name and version from a well-formed manifest", () => { + const extracted = fakeExtracted( + [manifestFile(JSON.stringify({ name: "lodash", version: "4.17.21" }))], + 1, + ); + const coordinate = readPackageManifest(extracted); + assert.deepEqual(coordinate, { name: "lodash", version: "4.17.21" }); + }); + + test("rejects duplicate package/package.json entries", () => { + // packageManifestEntryCount is the tar-entry count, tracked independently + // of the deduplicated `files` list — this is the shape a tarball with two + // manifest entries produces. + const extracted = fakeExtracted( + [manifestFile(JSON.stringify({ name: "lodash", version: "4.17.21" }))], + 2, + ); + assert.throws( + () => readPackageManifest(extracted), + /malformed npm tarball: duplicate package\/package\.json entries/, + ); + }); + + test("rejects a tarball with no readable manifest", () => { + const extracted = fakeExtracted([], 0); + assert.throws( + () => readPackageManifest(extracted), + /malformed npm tarball: missing readable package\/package\.json/, + ); + }); + + test("rejects a manifest that is not valid JSON", () => { + const extracted = fakeExtracted([manifestFile("{ not json")], 1); + assert.throws( + () => readPackageManifest(extracted), + /malformed npm tarball: package\/package\.json is not valid JSON/, + ); + }); + + test("rejects a manifest with no name", () => { + const extracted = fakeExtracted([manifestFile(JSON.stringify({ version: "1.0.0" }))], 1); + assert.throws( + () => readPackageManifest(extracted), + /malformed npm tarball: package\/package\.json declares no name/, + ); + }); + + test("rejects a manifest with no version", () => { + const extracted = fakeExtracted([manifestFile(JSON.stringify({ name: "lodash" }))], 1); + assert.throws( + () => readPackageManifest(extracted), + /malformed npm tarball: package\/package\.json declares no version/, + ); + }); +});