diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index cecd07fca..c4dd6bbf9 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -18,6 +18,34 @@ entry. See `CONTRIBUTING.md` § Releases & changelog. ## [Unreleased] +### Fixed — plugin ingest rejected the Teams app-package template (#860 W1a) + +2026-08-26 — A store update to `@omadia/channel-teams` 0.21.0 failed on the live +instance with `entry appPackage/manifest.json.template has a disallowed +extension (.template)`. Producer, gatekeeper and consumer are all in-house and +disagreed, so the agent factory could not install its template anywhere: + +- the published 0.21.0 artifact ships `appPackage/manifest.json.template` + (verified against the hub zip's central directory), +- `zipExtractor`'s `EXTENSION_ALLOWLIST` is deny-by-default and had no + `.template`, so ingest rejected the whole package, +- `teamsAppPackageAssets` reads exactly that filename and requires it by name. + +`.template` is now accepted, scoped to `appPackage/` — the same construction +`.woff2` uses under `ui/`. Scoped rather than global because the extension says +nothing about content: the file is read as text and never loaded or executed, +which puts it in the class of the already-allowed `.txt` / `.md`, but only the +one directory has a reason to carry it. Renaming to an allowed extension was the +alternative and is worse: the consumer name is compiled into the running +release, so a renamed plugin would blind the factory until the middleware caught +up — an ordering constraint for no security gain. + +The real gap was in the process, not the deploy: nothing ever held the published +package layout against the ingest gate (`npm run package` checks the zip builds, +the drift-guard checks versions). `pluginPackageTemplateAllowlist.test.ts` now +pushes the actual `appPackage/` layout through the extractor and pins the scope +in both directions. + ### Fixed — Teams answer card: honest badges and a Fresh-Check button that means something (#859, #878) 2026-08-25/26 — Field report on a bare `ping` → `Pong.` turn: the card claimed diff --git a/middleware/src/plugins/zipExtractor.ts b/middleware/src/plugins/zipExtractor.ts index fffdb2e11..d5b372b1e 100644 --- a/middleware/src/plugins/zipExtractor.ts +++ b/middleware/src/plugins/zipExtractor.ts @@ -68,6 +68,27 @@ const UI_BUNDLE_EXTENSIONS: ReadonlySet = new Set(['.woff2']); /** Matches `ui/...` at the root or below a single wrapper directory. */ const UI_BUNDLE_PATH = /(?:^|\/)ui\//; +/** + * Extensions accepted ONLY below an `appPackage/` directory (#860 W1a). + * + * `@omadia/channel-teams` ships the Teams app-package manifest as a TEMPLATE + * (`appPackage/manifest.json.template`); the agent factory reads it verbatim + * and substitutes the per-agent identity before zipping a real Teams app. The + * file is inert on our side — `teamsAppPackageAssets` reads it with + * `readFile(…, 'utf8')` and treats it as text; nothing loads or executes it, + * which puts it in the same class as the `.txt` / `.md` already allowed + * globally. + * + * Scoped rather than global for the same reason as {@link UI_BUNDLE_EXTENSIONS}: + * `.template` says nothing about content, so widening the whole ingest surface + * to accept it everywhere buys reach this feature does not need. One directory, + * one purpose. + */ +const APP_PACKAGE_EXTENSIONS: ReadonlySet = new Set(['.template']); + +/** Matches `appPackage/...` at the root or below a single wrapper directory. */ +const APP_PACKAGE_PATH = /(?:^|\/)appPackage\//; + export interface ExtractLimits { maxEntries: number; maxExtractedBytes: number; @@ -187,10 +208,14 @@ export async function extractZipToDir( baseName === '.npmignore'; const uiBundleAllowed = UI_BUNDLE_EXTENSIONS.has(ext) && UI_BUNDLE_PATH.test(relativeName); + const appPackageAllowed = + APP_PACKAGE_EXTENSIONS.has(ext) && + APP_PACKAGE_PATH.test(relativeName); if ( !EXTENSION_ALLOWLIST.has(ext) && !DECL_EXTENSIONS.has(ext) && !uiBundleAllowed && + !appPackageAllowed && !isTopLevelLike ) { throw new ZipExtractionError( diff --git a/middleware/test/pluginPackageTemplateAllowlist.test.ts b/middleware/test/pluginPackageTemplateAllowlist.test.ts new file mode 100644 index 000000000..a6f291a00 --- /dev/null +++ b/middleware/test/pluginPackageTemplateAllowlist.test.ts @@ -0,0 +1,212 @@ +/** + * `.template` in the plugin-package extension allowlist — scoped to `appPackage/`. + * + * `@omadia/channel-teams` ships the Teams app-package manifest as a template + * (`appPackage/manifest.json.template`) and the agent factory reads it verbatim + * to build a per-agent Teams app (#860 W1a). Without the entry the extractor + * rejects the whole package with `zip.forbidden_extension`, which is exactly + * what a live store update hit: producer, gatekeeper and consumer are all + * in-house and disagreed, so the feature could not install anywhere. + * + * The grant is deliberately SCOPED, mirroring `.woff2` under `ui/`: `.template` + * says nothing about content, so it is admitted in the one directory that has a + * reason to carry it and nowhere else. These tests pin the scope, not just the + * happy path — if `.template` ever starts extracting outside `appPackage/`, the + * narrowing has been lost. + */ + +import { describe, it, beforeEach, afterEach } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { promises as fs } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { PackageUploadService } from '../src/plugins/packageUploadService.js'; +import { extractZipToDir, ZipExtractionError } from '../src/plugins/zipExtractor.js'; +import { + buildZip, + fakeCatalog, + fakeStore, + packageZip, +} from './_helpers/pluginPackageZip.js'; + +/** Shape of the real file: JSON with the placeholders the factory substitutes. */ +const MANIFEST_TEMPLATE = JSON.stringify( + { id: '{{AGENT_APP_ID}}', name: { short: '{{AGENT_NAME}}' } }, + null, + 2, +); + +// --------------------------------------------------------------------------- +// Extractor-level — the allowlist entry and its scope +// --------------------------------------------------------------------------- + +describe('zipExtractor × .template', () => { + let root: string; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'zip-template-')); + }); + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + async function extract( + files: Record, + overrides: Partial[2]> = {}, + ): Promise { + const zipPath = path.join(root, 'in.zip'); + await fs.writeFile(zipPath, await buildZip(files)); + const result = await extractZipToDir(zipPath, path.join(root, 'out'), { + maxEntries: 50, + maxExtractedBytes: 1024 * 1024, + ...overrides, + }); + return result.files; + } + + it('accepts appPackage/manifest.json.template under the default allowlist', async () => { + const files = await extract({ + 'appPackage/manifest.json.template': MANIFEST_TEMPLATE, + }); + assert.deepEqual(files, ['appPackage/manifest.json.template']); + assert.equal( + await fs.readFile( + path.join(root, 'out', 'appPackage', 'manifest.json.template'), + 'utf8', + ), + MANIFEST_TEMPLATE, + ); + }); + + // The scope IS the security property — a global `.template` grant would let + // any package drop an arbitrarily-named blob anywhere in its tree. + it('rejects .template at the package root', async () => { + await assert.rejects( + () => extract({ 'manifest.json.template': MANIFEST_TEMPLATE }), + (err: unknown) => { + assert.ok(err instanceof ZipExtractionError); + assert.equal(err.code, 'zip.forbidden_extension'); + return true; + }, + ); + }); + + it('rejects .template in a directory that merely looks similar', async () => { + await assert.rejects( + () => extract({ 'appPackages/manifest.json.template': MANIFEST_TEMPLATE }), + (err: unknown) => + err instanceof ZipExtractionError && err.code === 'zip.forbidden_extension', + ); + }); + + it('accepts appPackage/ below a single wrapper directory', async () => { + // `npm pack`-style zips wrap everything in one top-level folder; the + // sibling `ui/` rule tolerates that and this one has to match it. + const files = await extract({ + 'channel-teams/appPackage/manifest.json.template': MANIFEST_TEMPLATE, + }); + assert.deepEqual(files, ['channel-teams/appPackage/manifest.json.template']); + }); + + // An explicit allowlist replaces the default set wholesale (the Profile-Bundle + // importer relies on that), so it must not silently inherit `.template`. + it('rejects .template when an explicit extensionAllowlist omits it', async () => { + await assert.rejects( + () => + extract( + { 'appPackage/manifest.json.template': MANIFEST_TEMPLATE }, + { extensionAllowlist: new Set(['.json', '.yaml']) }, + ), + (err: unknown) => + err instanceof ZipExtractionError && err.code === 'zip.forbidden_extension', + ); + }); + + it('still rejects an executable extension inside appPackage/', async () => { + await assert.rejects( + () => extract({ 'appPackage/run.sh': '#!/bin/sh\n' }), + (err: unknown) => + err instanceof ZipExtractionError && err.code === 'zip.forbidden_extension', + ); + }); + + // The regression that would have caught this before it reached a live store + // update: nothing ever held the PUBLISHED package layout against the ingest + // gate. `npm run package` checks the zip is built and the drift-guard checks + // versions, but neither asks "does this zip get past the extractor". These + // are the four entries the 0.21.0 artifact actually carries under + // `appPackage/` (verified against the hub zip's central directory), and + // `teamsAppPackageAssets` requires the first three by name. + it('accepts the published channel-teams appPackage/ layout as a whole', async () => { + const files = await extract({ + 'appPackage/manifest.json.template': MANIFEST_TEMPLATE, + // Extension-gated, so byte-accurate PNGs would prove nothing extra here. + 'appPackage/color.png': 'PNG', + 'appPackage/outline.png': 'PNG', + 'appPackage/README.md': '# app package\n', + }); + assert.deepEqual(files.sort(), [ + 'appPackage/README.md', + 'appPackage/color.png', + 'appPackage/manifest.json.template', + 'appPackage/outline.png', + ]); + }); +}); + +// --------------------------------------------------------------------------- +// Ingest-level — the store update that actually failed +// --------------------------------------------------------------------------- + +describe('PackageUploadService ingest × the Teams app-package layout', () => { + let root: string; + let packagesDir: string; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'upload-template-')); + packagesDir = path.join(root, '.uploaded-packages'); + await fs.mkdir(packagesDir, { recursive: true }); + }); + + afterEach(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + function service(): PackageUploadService { + return new PackageUploadService({ + store: fakeStore(), + catalog: fakeCatalog(), + packagesDir, + limits: { + maxBytes: 1024 * 1024, + maxExtractedBytes: 4 * 1024 * 1024, + maxEntries: 50, + }, + hostDependencies: {}, + log: () => undefined, + }); + } + + it('installs a channel package shipping appPackage/{template,icons}', async () => { + const result = await service().ingest({ + fileBuffer: await packageZip('@omadia/channel-teams', '0.21.0', { + 'appPackage/manifest.json.template': MANIFEST_TEMPLATE, + 'appPackage/README.md': '# app package\n', + }), + originalFilename: 'channel-teams.zip', + uploadedBy: 'operator@example.com', + }); + + assert.equal(result.ok, true); + const finalDir = path.join(packagesDir, '@omadia', 'channel-teams', '0.21.0'); + assert.equal( + await fs.readFile( + path.join(finalDir, 'appPackage', 'manifest.json.template'), + 'utf8', + ), + MANIFEST_TEMPLATE, + ); + }); +});