From 8e28e29c26773013f0d36706d0caaaa1f7cc1f96 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 26 May 2026 16:29:47 +0900 Subject: [PATCH] fix(package): block internal-only assets from publish payload Validator finding #12 from the publish-debate-vortex hyperultradebate flagged that internal-only skill and command assets could leak into the npm payload once the dot asset roots are included. Bun 1.3.x ignores a root .npmignore for directories listed in package.json#files, so the exclusion rules live in nested .npmignore files co-located with each published command and skill directory. RED before nested ignores: bun test script/package-layout-exclusion.test.ts failed with expect(received).toEqual(expected), receiving .opencode/skills/__internal-fake-do-not-ship-test-artifact/SKILL.md, .agents/skills/__internal-fake-do-not-ship-test-artifact/SKILL.md, .opencode/command/__internal-fake-do-not-ship-test-artifact.md, and .agents/command/__internal-fake-do-not-ship-test-artifact.md instead of []. GREEN after nested ignores: bun test script/package-layout-exclusion.test.ts reported 2 pass, 0 fail, 5 expect() calls. This is the exclusion companion to script/package-layout.test.ts, the inclusion test arriving through the dev merge. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .agents/command/.npmignore | 10 ++ .agents/skills/.npmignore | 10 ++ .opencode/command/.npmignore | 10 ++ .opencode/skills/.npmignore | 10 ++ script/package-layout-exclusion.test.ts | 208 ++++++++++++++++++++++++ 5 files changed, 248 insertions(+) create mode 100644 .agents/command/.npmignore create mode 100644 .agents/skills/.npmignore create mode 100644 .opencode/command/.npmignore create mode 100644 .opencode/skills/.npmignore create mode 100644 script/package-layout-exclusion.test.ts diff --git a/.agents/command/.npmignore b/.agents/command/.npmignore new file mode 100644 index 000000000..6524d100e --- /dev/null +++ b/.agents/command/.npmignore @@ -0,0 +1,10 @@ +# Internal-only assets — never ship to npm registry. +# See script/package-layout-exclusion.test.ts for the enforcing guard. +# Root .npmignore does not work for directories listed in package.json#files +# under Bun 1.3.x, so the guard lives co-located with the published content. +__*/ +__*.md +.private/ +.draft/ +.private.md +.draft.md diff --git a/.agents/skills/.npmignore b/.agents/skills/.npmignore new file mode 100644 index 000000000..6524d100e --- /dev/null +++ b/.agents/skills/.npmignore @@ -0,0 +1,10 @@ +# Internal-only assets — never ship to npm registry. +# See script/package-layout-exclusion.test.ts for the enforcing guard. +# Root .npmignore does not work for directories listed in package.json#files +# under Bun 1.3.x, so the guard lives co-located with the published content. +__*/ +__*.md +.private/ +.draft/ +.private.md +.draft.md diff --git a/.opencode/command/.npmignore b/.opencode/command/.npmignore new file mode 100644 index 000000000..6524d100e --- /dev/null +++ b/.opencode/command/.npmignore @@ -0,0 +1,10 @@ +# Internal-only assets — never ship to npm registry. +# See script/package-layout-exclusion.test.ts for the enforcing guard. +# Root .npmignore does not work for directories listed in package.json#files +# under Bun 1.3.x, so the guard lives co-located with the published content. +__*/ +__*.md +.private/ +.draft/ +.private.md +.draft.md diff --git a/.opencode/skills/.npmignore b/.opencode/skills/.npmignore new file mode 100644 index 000000000..6524d100e --- /dev/null +++ b/.opencode/skills/.npmignore @@ -0,0 +1,10 @@ +# Internal-only assets — never ship to npm registry. +# See script/package-layout-exclusion.test.ts for the enforcing guard. +# Root .npmignore does not work for directories listed in package.json#files +# under Bun 1.3.x, so the guard lives co-located with the published content. +__*/ +__*.md +.private/ +.draft/ +.private.md +.draft.md diff --git a/script/package-layout-exclusion.test.ts b/script/package-layout-exclusion.test.ts new file mode 100644 index 000000000..1fb0e37d3 --- /dev/null +++ b/script/package-layout-exclusion.test.ts @@ -0,0 +1,208 @@ +/// + +import { afterAll, beforeAll, describe, expect, test } from "bun:test" +import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs" +import { dirname, join, relative, sep } from "node:path" +import { fileURLToPath } from "node:url" + +const repositoryRoot = fileURLToPath(new URL("..", import.meta.url)) +const packageJsonPath = join(repositoryRoot, "package.json") +const fakeArtifactName = "__internal-fake-do-not-ship-test-artifact" +const packageAssetRoots = [".opencode/command", ".opencode/skills", ".agents/command", ".agents/skills"] as const +const fakeInternalSkillArtifactRootPaths = [ + `.opencode/skills/${fakeArtifactName}`, + `.agents/skills/${fakeArtifactName}`, +] as const +const fakeInternalSkillArtifactPaths = [ + `${fakeInternalSkillArtifactRootPaths[0]}/SKILL.md`, + `${fakeInternalSkillArtifactRootPaths[1]}/SKILL.md`, +] as const +const fakeInternalCommandArtifactPaths = [ + `.opencode/command/${fakeArtifactName}.md`, + `.agents/command/${fakeArtifactName}.md`, +] as const +const fakeInternalArtifactCleanupPaths = [ + ...fakeInternalSkillArtifactRootPaths, + ...fakeInternalCommandArtifactPaths, +] as const + +let originalPackageJsonText: string | null = null +let packageJsonWasTemporarilyModified = false + +class PackDryRunError extends Error { + constructor(readonly exitCode: number, readonly stderr: string) { + super(`bun pm pack --dry-run failed with exit code ${exitCode}: ${stderr}`) + this.name = "PackDryRunError" + } +} + +class PackageFilesAnchorError extends Error { + constructor() { + super("package.json files list no longer contains the postinstall.mjs anchor") + this.name = "PackageFilesAnchorError" + } +} + +function toPackagePath(filePath: string): string { + return relative(repositoryRoot, filePath).split(sep).join("/") +} + +function collectPackagePathsRecursively(rootPath: string): string[] { + const collectedPaths: string[] = [] + const directories = [rootPath] + + while (directories.length > 0) { + const currentDirectory = directories.pop() + if (!currentDirectory) { + continue + } + + for (const entry of readdirSync(currentDirectory, { withFileTypes: true })) { + const entryPath = join(currentDirectory, entry.name) + if (entry.isDirectory()) { + directories.push(entryPath) + continue + } + + if (entry.isFile()) { + collectedPaths.push(toPackagePath(entryPath)) + } + } + } + + return collectedPaths +} + +function parsePackedPaths(output: string): Set { + const packedPaths = new Set() + const packedPathPattern = /^packed\s+\S+\s+(.+)$/ + + for (const line of output.split("\n")) { + const match = packedPathPattern.exec(line) + const packedPath = match?.at(1) + if (packedPath) { + packedPaths.add(packedPath) + } + } + + return packedPaths +} + +async function packDryRunPaths(): Promise> { + const packProcess = Bun.spawn({ + cmd: ["bun", "pm", "pack", "--dry-run"], + cwd: repositoryRoot, + stdout: "pipe", + stderr: "pipe", + }) + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(packProcess.stdout).text(), + new Response(packProcess.stderr).text(), + packProcess.exited, + ]) + + if (exitCode !== 0) { + throw new PackDryRunError(exitCode, stderr) + } + + return parsePackedPaths(stdout) +} + +function withPackageAssetRoots(packageJsonText: string): string { + const missingAssetRoots = packageAssetRoots.filter((rootPath) => !packageJsonText.includes(`"${rootPath}"`)) + if (missingAssetRoots.length === 0) { + return packageJsonText + } + + const filesAnchor = ' "postinstall.mjs",\n' + if (!packageJsonText.includes(filesAnchor)) { + throw new PackageFilesAnchorError() + } + + const insertedAssetRoots = missingAssetRoots.map((rootPath) => ` "${rootPath}",`).join("\n") + return packageJsonText.replace(filesAnchor, `${filesAnchor}${insertedAssetRoots}\n`) +} + +function preparePackageJsonForDotAssetPacking(): void { + const packageJsonText = readFileSync(packageJsonPath, "utf8") + originalPackageJsonText = packageJsonText + const packageJsonTextWithAssetRoots = withPackageAssetRoots(packageJsonText) + packageJsonWasTemporarilyModified = packageJsonTextWithAssetRoots !== packageJsonText + + if (packageJsonWasTemporarilyModified) { + writeFileSync(packageJsonPath, packageJsonTextWithAssetRoots) + } +} + +function restorePackageJson(): void { + if (packageJsonWasTemporarilyModified && originalPackageJsonText !== null) { + writeFileSync(packageJsonPath, originalPackageJsonText) + } +} + +function removeFakeInternalArtifacts(): void { + for (const packagePath of fakeInternalArtifactCleanupPaths) { + rmSync(join(repositoryRoot, packagePath), { recursive: true, force: true }) + } +} + +function writeFakeInternalArtifacts(packagePaths: readonly string[]): void { + for (const packagePath of packagePaths) { + const artifactPath = join(repositoryRoot, packagePath) + mkdirSync(dirname(artifactPath), { recursive: true }) + writeFileSync(artifactPath, "# Fake internal artifact for package-layout-exclusion.test.ts\n") + } +} + +function collectExistingFakeInternalSkillArtifactPaths(): string[] { + return fakeInternalSkillArtifactRootPaths + .filter((packagePath) => existsSync(join(repositoryRoot, packagePath))) + .flatMap((packagePath) => collectPackagePathsRecursively(join(repositoryRoot, packagePath))) + .sort() +} + +describe("published package layout exclusions", () => { + beforeAll(() => { + removeFakeInternalArtifacts() + + try { + preparePackageJsonForDotAssetPacking() + writeFakeInternalArtifacts([...fakeInternalSkillArtifactPaths, ...fakeInternalCommandArtifactPaths]) + } catch (error) { + removeFakeInternalArtifacts() + restorePackageJson() + throw error + } + }) + + afterAll(() => { + removeFakeInternalArtifacts() + restorePackageJson() + }) + + test("#given internal-only skill assets #when packing package #then forbidden skill assets do not ship", async () => { + // given + expect(collectExistingFakeInternalSkillArtifactPaths()).toEqual(fakeInternalSkillArtifactPaths.toSorted()) + + // when + const packedPaths = await packDryRunPaths() + + // then + const packedInternalSkillPaths = fakeInternalSkillArtifactPaths.filter((packagePath) => packedPaths.has(packagePath)) + expect(packedInternalSkillPaths).toEqual([]) + }) + + test("#given internal-only command assets #when packing package #then forbidden command assets do not ship", async () => { + // given + for (const packagePath of fakeInternalCommandArtifactPaths) { + expect(existsSync(join(repositoryRoot, packagePath))).toBe(true) + } + + // when + const packedPaths = await packDryRunPaths() + + // then + const packedInternalCommandPaths = fakeInternalCommandArtifactPaths.filter((packagePath) => packedPaths.has(packagePath)) + expect(packedInternalCommandPaths).toEqual([]) + }) +})