From 9d73d0c93586edd88f5ed386ba68cc22b3d7f4f0 Mon Sep 17 00:00:00 2001 From: agarciar Date: Mon, 25 May 2026 10:56:06 +0200 Subject: [PATCH 1/3] fix(walk): translate ** and ? globs correctly matchesGlob expanded the glob through successive .replace() calls, so turning ** into `.*` (and `**/` into `(?:.*/)?`) injected `*` characters that the later single-`*` pass reprocessed, corrupting `.*` into `.[^/]*`. As a result ** only matched a single path segment and `?` acted as a regex quantifier. Rewrite as a single-pass alternation and escape `?` as a literal, matching the documented "**, *, and literals" feature set. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/walk.spec.ts | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ src/walk.ts | 25 +++++++++++++----- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/walk.spec.ts b/src/walk.spec.ts index 41ebb8a..5026e80 100644 --- a/src/walk.spec.ts +++ b/src/walk.spec.ts @@ -45,3 +45,72 @@ describe('walkLocal', () => { expect(await walkLocal(dir, [])).toEqual([]); }); }); + +describe('walkLocal ignore glob semantics', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'bunny-glob-')); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it('** under a prefix matches files at any depth', async () => { + writeFileSync(join(dir, 'keep.js'), 'a'); + mkdirSync(join(dir, 'assets')); + writeFileSync(join(dir, 'assets', 'top.js'), 'a'); + mkdirSync(join(dir, 'assets', 'deep')); + writeFileSync(join(dir, 'assets', 'deep', 'nested.js'), 'a'); + + const files = await walkLocal(dir, ['assets/**']); + + expect(files.map((f) => f.relPath)).toEqual(['keep.js']); + }); + + it('leading **/ matches the trailing pattern at any depth', async () => { + writeFileSync(join(dir, 'root.map'), 'a'); + mkdirSync(join(dir, 'a')); + writeFileSync(join(dir, 'a', 'one.map'), 'a'); + mkdirSync(join(dir, 'a', 'b')); + writeFileSync(join(dir, 'a', 'b', 'two.map'), 'a'); + writeFileSync(join(dir, 'a', 'b', 'keep.js'), 'a'); + + const files = await walkLocal(dir, ['**/*.map']); + + expect(files.map((f) => f.relPath)).toEqual(['a/b/keep.js']); + }); + + it('a middle **/ spans zero or more directories', async () => { + mkdirSync(join(dir, 'a')); + writeFileSync(join(dir, 'a', 'b.js'), 'a'); + mkdirSync(join(dir, 'a', 'x')); + writeFileSync(join(dir, 'a', 'x', 'b.js'), 'a'); + writeFileSync(join(dir, 'keep.txt'), 'a'); + + const files = await walkLocal(dir, ['a/**/b.js']); + + expect(files.map((f) => f.relPath)).toEqual(['keep.txt']); + }); + + it('a single * does not cross directory boundaries', async () => { + writeFileSync(join(dir, 'app.js'), 'a'); + mkdirSync(join(dir, 'sub')); + writeFileSync(join(dir, 'sub', 'app.js'), 'a'); + + const files = await walkLocal(dir, ['*.js']); + + expect(files.map((f) => f.relPath)).toEqual(['sub/app.js']); + }); + + it('treats ? as a literal, not a single-char wildcard', async () => { + // The glob feature set is documented as "**, *, and literals"; ? must not + // behave like a regex quantifier and make the preceding char optional. + writeFileSync(join(dir, 'chunk.js'), 'a'); + + const files = await walkLocal(dir, ['chunk-?.js']); + + expect(files.map((f) => f.relPath)).toEqual(['chunk.js']); + }); +}); diff --git a/src/walk.ts b/src/walk.ts index 08ce199..8c76fb4 100644 --- a/src/walk.ts +++ b/src/walk.ts @@ -9,12 +9,25 @@ function toPosix(p: string): string { } function matchesGlob(relPath: string, glob: string): boolean { - const escaped = glob - .replace(/[.+^${}()|[\]\\]/g, '\\$&') - .replace(/^\*\*\//, '(?:.*/)?') // leading **/ → optional directory prefix - .replace(/\*\*/g, '.*') // remaining ** → any chars including / - .replace(/\*/g, '[^/]*'); // single * → any non-slash chars - return new RegExp('^' + escaped + '$').test(relPath); + // Translate the glob to a regex in a SINGLE pass. Doing it in successive + // .replace() calls is unsafe: expanding ** into `.*` (or `**/` into + // `(?:.*/)?`) injects `*` characters that a later single-* pass would + // reprocess, corrupting `.*` into `.[^/]*` and breaking deep matching. + // Alternation order matters — `**/` and `**` are matched before a lone `*`. + // Everything else (including `?`) is a literal and gets escaped. + const pattern = glob.replace(/\*\*\/|\*\*|\*|[.+^${}()|[\]?\\]/g, (token) => { + switch (token) { + case '**/': + return '(?:.*/)?'; // any number of leading directories, including none + case '**': + return '.*'; // any characters, including / + case '*': + return '[^/]*'; // any run of non-slash characters + default: + return `\\${token}`; // escape a regex metacharacter → literal match + } + }); + return new RegExp(`^${pattern}$`).test(relPath); } function isIgnored(relPath: string, ignore: string[]): boolean { From 55aaf1e85d5a5178df947dc16f1773c35082002e Mon Sep 17 00:00:00 2001 From: agarciar Date: Mon, 25 May 2026 10:56:06 +0200 Subject: [PATCH 2/3] fix(deploy): resolve relative outputPath and refuse mass deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveOutputPath returned a relative outputPath verbatim while the buildTarget branch resolved it against workspaceRoot, so a relative path resolved against cwd and could read the wrong (or missing) folder. Resolve both branches against workspaceRoot via a shared helper. Add a safety floor that refuses to proceed when the local folder is empty but the remote is not — which would otherwise mirror into deleting the entire site. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/deploy.spec.ts | 33 +++++++++++++++++++++++++++++++++ src/deploy.ts | 21 +++++++++++++++++---- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/deploy.spec.ts b/src/deploy.spec.ts index 479ded0..f5d3316 100644 --- a/src/deploy.spec.ts +++ b/src/deploy.spec.ts @@ -184,4 +184,37 @@ describe('runDeploy', () => { expect(getTargetOptions).toHaveBeenCalled(); expect(client.upload).toHaveBeenCalledTimes(2); }); + + it('refuses to delete the whole remote when the local folder is empty', async () => { + const emptyDir = mkdtempSync(join(tmpdir(), 'bunny-empty-')); + try { + client.listAll.mockResolvedValue([ + { relPath: 'index.html', size: 15, sha256: 'abc' }, + { relPath: 'app.js', size: 3, sha256: 'def' }, + ]); + const out = await runDeploy(baseOptions({ outputPath: emptyDir }), fakeContext(), deps); + expect(out.success).toBe(false); + expect(out.error).toMatch(/empty/i); + expect(client.remove).not.toHaveBeenCalled(); + expect(client.upload).not.toHaveBeenCalled(); + expect(client.purgePullZone).not.toHaveBeenCalled(); + } finally { + rmSync(emptyDir, { recursive: true, force: true }); + } + }); + + it('resolves a relative outputPath against the workspace root, not cwd', async () => { + const wsRoot = mkdtempSync(join(tmpdir(), 'bunny-ws-')); + const browserDirName = 'relative-out'; + mkdirSync(join(wsRoot, browserDirName)); + writeFileSync(join(wsRoot, browserDirName, 'index.html'), ''); + try { + const ctx = fakeContext({ workspaceRoot: wsRoot }); + const out = await runDeploy(baseOptions({ outputPath: browserDirName }), ctx, deps); + expect(out.success).toBe(true); + expect(client.upload).toHaveBeenCalledTimes(1); + } finally { + rmSync(wsRoot, { recursive: true, force: true }); + } + }); }); diff --git a/src/deploy.ts b/src/deploy.ts index eb49ecc..da913e3 100644 --- a/src/deploy.ts +++ b/src/deploy.ts @@ -45,11 +45,16 @@ function formatBytes(n: number): string { return `${(n / (1024 * 1024)).toFixed(1)} MB`; } +function toAbsolute(p: string, workspaceRoot: string): string { + const isAbs = p.startsWith('/') || /^[A-Za-z]:[\\/]/.test(p); + return isAbs ? p : join(workspaceRoot, p); +} + async function resolveOutputPath( options: DeployOptions, context: BuilderContext, ): Promise { - if (options.outputPath) return options.outputPath; + if (options.outputPath) return toAbsolute(options.outputPath, context.workspaceRoot); if (!options.buildTarget) { throw new Error('Either buildTarget or outputPath must be set.'); } @@ -82,9 +87,7 @@ async function resolveOutputPath( } else { baseDir = join('dist', projectName); } - const isAbs = baseDir.startsWith('/') || /^[A-Za-z]:[\\/]/.test(baseDir); - const abs = isAbs ? baseDir : join(context.workspaceRoot, baseDir); - return join(abs, browserDir); + return join(toAbsolute(baseDir, context.workspaceRoot), browserDir); } export async function runDeploy( @@ -131,6 +134,16 @@ export async function runDeploy( `Diff: ${plan.toUpload.length} upload (${formatBytes(uploadBytes)}), ${plan.toDelete.length} delete, ${plan.unchanged.length} unchanged.`, ); + // Safety floor: an empty local folder against a non-empty remote would mirror + // into deleting the entire site. That almost always means the build produced + // no output or outputPath is wrong — refuse before any destructive call. + if (local.length === 0 && remote.length > 0) { + return { + success: false, + error: `Refusing to delete all ${remote.length} remote file(s): the local output folder "${outputPath}" is empty. Check that the build produced output and that buildTarget/outputPath is correct.`, + }; + } + if (options.dryRun) { for (const f of plan.toUpload) log.debug(` + ${f.relPath} (${formatBytes(f.size)})`); for (const f of plan.toDelete) log.debug(` - ${f.relPath}`); From 8f7c6648a7ced645770f6f0e3e6e5cf2efa18eea Mon Sep 17 00:00:00 2001 From: agarciar Date: Mon, 25 May 2026 10:56:06 +0200 Subject: [PATCH 3/3] ci(schema): require pullZoneId >= 1 and smoke-test the dist entry point Add minimum:1 to pullZoneId so `--pull-zone-id 0` is rejected at parse time instead of producing a swallowed 404 purge. Build and import dist/deploy.js in both CI jobs to prove the published artifact loads and exposes the builder against Node 22/24 and Angular 17-21. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 12 ++++++++++++ src/schema.json | 1 + 2 files changed, 13 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 33d3325..4c3bd68 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,12 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm run typecheck - run: pnpm test + - run: pnpm run build + # The suite imports src/*.ts; this proves the published dist/ entry point + # actually loads and exposes the builder (catches a broken/missing emit). + - name: Smoke-test the published entry point + run: | + node -e "import('./dist/deploy.js').then((m) => { if (typeof m.default === 'undefined' || typeof m.runDeploy !== 'function') { console.error('dist/deploy.js is missing expected exports'); process.exit(1); } }).catch((e) => { console.error(e); process.exit(1); })" # Verifies the @angular-devkit/architect peer surface compiles and the suite # passes against every supported Angular major (the peerDependencies range). @@ -60,3 +66,9 @@ jobs: - run: pnpm add -D --ignore-scripts "@angular-devkit/architect@${{ matrix.architect }}" "@angular-devkit/core@${{ matrix.core }}" - run: pnpm run typecheck - run: pnpm test + - run: pnpm run build + # Confirms dist/deploy.js loads at runtime against this Angular major — + # guards the CJS/ESM interop of the @angular-devkit peer import. + - name: Smoke-test the published entry point + run: | + node -e "import('./dist/deploy.js').then((m) => { if (typeof m.default === 'undefined' || typeof m.runDeploy !== 'function') { console.error('dist/deploy.js is missing expected exports'); process.exit(1); } }).catch((e) => { console.error(e); process.exit(1); })" diff --git a/src/schema.json b/src/schema.json index 6bda350..14c17b7 100644 --- a/src/schema.json +++ b/src/schema.json @@ -31,6 +31,7 @@ "pullZoneId": { "type": ["number", "null"], "default": null, + "minimum": 1, "description": "Numeric Pull Zone ID to purge after upload. Required when purgeAfterUpload is true." }, "purgeAfterUpload": {