Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions middleware/src/plugins/zipExtractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@ const UI_BUNDLE_EXTENSIONS: ReadonlySet<string> = 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<string> = 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;
Expand Down Expand Up @@ -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(
Expand Down
212 changes: 212 additions & 0 deletions middleware/test/pluginPackageTemplateAllowlist.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, string>,
overrides: Partial<Parameters<typeof extractZipToDir>[2]> = {},
): Promise<string[]> {
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,
);
});
});
Loading