refactor(agent-roles-catalog): store catalog as YAML with block-scalar instructions (#229)

The agent-roles catalog content files move from JSON to YAML so each role's long `instructions` system prompt is stored as a literal block scalar (`|-`): editing one sentence now produces a line-by-line diff and the prompt is editable as plain multi-line text instead of a single escaped JSON string. Data: - `index.json` -> `index.yaml`, `bundles/<id>/<lang>.json` -> `<lang>.yaml` (old `.json` deleted). Converted programmatically via the `yaml` library with `lineWidth: 0`; round-trip verified deepEqual against the old JSON, so the resolved role content is byte-for-byte identical and no `version` is bumped (content-hash lock unchanged). Server (`AiAgentRolesCatalogProvider`): - parse with `yaml`'s safe default (JSON-compatible) schema instead of `JSON.parse` — `strict: true` (rejects unknown tags / duplicate keys) and `maxAliasCount: 100` (billion-laughs guard); no custom `!!` tags / no code execution. Fetched paths become `index.yaml` / `<lang>.yaml`. The streaming 1 MB size cap, `redirect: 'error'`, 10s timeout and `^[a-z0-9-]+$` path-traversal/SSRF guard are unchanged; the hand-written type guards are untouched (`instructions` is still a string after parsing). - add `yaml` as a direct server dependency (already in the lockfile as a transitive dep). Catalog tooling: - `scripts/check.mjs` parses the catalog as YAML (lockfile stays JSON); pin `yaml` as a devDependency of the catalog package. Tests: - provider spec fixtures serialized with `yaml`; new tests for the block-scalar `instructions` round-trip (exact multi-line string), malformed YAML and strict duplicate-key rejection -> BadGateway; size-cap and path-traversal cases retargeted to the `.yaml` paths. Docs: README, `.env.example`, `catalog-types.ts` comments and CHANGELOG updated to the YAML layout. `AI_AGENT_ROLES_CATALOG_URL` base-URL contract unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-27 06:27:48 +03:00
parent 904f7b4303
commit 0ec954b382
20 changed files with 1087 additions and 280 deletions
@@ -125,6 +125,7 @@
    "typesense": "^3.0.5",
    "undici": "7.24.0",
    "ws": "^8.20.1",
+    "yaml": "^2.8.3",
    "yauzl": "^3.2.1",
    "zod": "^4.3.6"
  },
@@ -1,12 +1,16 @@
 import { BadGatewayException, BadRequestException } from '@nestjs/common';
+import { stringify as stringifyYaml } from 'yaml';
 import { AiAgentRolesCatalogProvider } from './ai-agent-roles-catalog.provider';

 /**
 * Provider tests against a mocked remote source (no network). They cover the
- * happy read path (fetchIndex / fetchBundle), the malformed-shape rejection,
- * rejection of non-http(s) sources (local sources are gone), and — most
- * importantly — the `^[a-z0-9-]+$` path-traversal guard that runs BEFORE any
- * path/URL is built.
+ * happy read path (fetchIndex / fetchBundle) over the YAML catalog format, the
+ * block-scalar `instructions` round-trip, the malformed-shape rejection, the
+ * malformed-YAML rejection, rejection of non-http(s) sources (local sources are
+ * gone), and — most importantly — the `^[a-z0-9-]+$` path-traversal guard that
+ * runs BEFORE any path/URL is built. Fixtures are serialized with the same
+ * `yaml` library the provider parses with (`stringifyYaml`), so the tests
+ * exercise real YAML, not the JSON subset.
 */
 describe('AiAgentRolesCatalogProvider', () => {
  function makeProvider(source: string) {
@@ -71,7 +75,7 @@ describe('AiAgentRolesCatalogProvider', () => {
    }

    it('fetchBundle remote happy path => parses + validates', async () => {
-      const json = JSON.stringify({
+      const yaml = stringifyYaml({
        schemaVersion: 1,
        language: 'en',
        roles: [
@@ -82,7 +86,7 @@ describe('AiAgentRolesCatalogProvider', () => {
          },
        ],
      });
-      const body = streamOf([new TextEncoder().encode(json)]);
+      const body = streamOf([new TextEncoder().encode(yaml)]);
      global.fetch = jest
        .fn()
        .mockResolvedValue(mockResponse({ body })) as never;
@@ -92,12 +96,12 @@ describe('AiAgentRolesCatalogProvider', () => {
    });

    it('fetchBundle remote malformed (role missing instructions) => BadGateway', async () => {
-      const json = JSON.stringify({
+      const yaml = stringifyYaml({
        schemaVersion: 1,
        language: 'fr',
        roles: [{ slug: 'researcher', name: 'Chercheur' }],
      });
-      const body = streamOf([new TextEncoder().encode(json)]);
+      const body = streamOf([new TextEncoder().encode(yaml)]);
      global.fetch = jest
        .fn()
        .mockResolvedValue(mockResponse({ body })) as never;
@@ -153,8 +157,9 @@ describe('AiAgentRolesCatalogProvider', () => {
        );
      global.fetch = fetchMock as never;
      const provider = makeProvider('https://catalog.example.com');
-      // Body shape is irrelevant; an empty stream parses to invalid JSON and
-      // throws, but the fetch call (with its init) still happened.
+      // Body shape is irrelevant; an empty stream parses to an empty YAML doc
+      // (null), fails the shape guard and throws, but the fetch call (with its
+      // init) still happened.
      await expect(provider.fetchIndex()).rejects.toBeDefined();
      expect(fetchMock).toHaveBeenCalledWith(
        expect.any(String),
@@ -190,7 +195,7 @@ describe('AiAgentRolesCatalogProvider', () => {
    });

    it('small streamed body parses normally (cap not hit)', async () => {
-      const json = JSON.stringify({
+      const yaml = stringifyYaml({
        schemaVersion: 1,
        bundles: [
          {
@@ -201,7 +206,7 @@ describe('AiAgentRolesCatalogProvider', () => {
          },
        ],
      });
-      const body = streamOf([new TextEncoder().encode(json)]);
+      const body = streamOf([new TextEncoder().encode(yaml)]);
      global.fetch = jest
        .fn()
        .mockResolvedValue(mockResponse({ body })) as never;
@@ -227,7 +232,7 @@ describe('AiAgentRolesCatalogProvider', () => {
    });

    it('null body (no readable stream) => response.text() fallback parses', async () => {
-      const json = JSON.stringify({
+      const yaml = stringifyYaml({
        schemaVersion: 1,
        bundles: [
          {
@@ -240,7 +245,7 @@ describe('AiAgentRolesCatalogProvider', () => {
      });
      global.fetch = jest
        .fn()
-        .mockResolvedValue(mockResponse({ body: null, text: json })) as never;
+        .mockResolvedValue(mockResponse({ body: null, text: yaml })) as never;
      const provider = makeProvider('https://catalog.example.com');
      const index = await provider.fetchIndex();
      expect(index.bundles[0].id).toBe('general');
@@ -259,8 +264,12 @@ describe('AiAgentRolesCatalogProvider', () => {
      );
    });

-    it('invalid JSON body => BadGateway (parse failure)', async () => {
-      const body = streamOf([new TextEncoder().encode('{not valid json')]);
+    it('invalid YAML body => BadGateway (parse failure)', async () => {
+      // An unterminated flow mapping is not valid YAML, so YAML.parse throws and
+      // the provider maps it to BadGateway (not a generic 500).
+      const body = streamOf([
+        new TextEncoder().encode('schemaVersion: {not: closed'),
+      ]);
      global.fetch = jest
        .fn()
        .mockResolvedValue(mockResponse({ body })) as never;
@@ -270,11 +279,28 @@ describe('AiAgentRolesCatalogProvider', () => {
      );
    });

-    it('malformed index.json (valid JSON, wrong shape) => BadGateway', async () => {
-      // Parses as JSON but fails isCatalogIndex (schemaVersion not a number).
+    it('YAML with a duplicate key (strict) => BadGateway (parse failure)', async () => {
+      // strict:true rejects duplicate mapping keys rather than last-wins coercing
+      // them — a defensive parse on untrusted input.
      const body = streamOf([
        new TextEncoder().encode(
-          JSON.stringify({ schemaVersion: 'x', bundles: [] }),
+          'schemaVersion: 1\nbundles: []\nschemaVersion: 2\n',
+        ),
+      ]);
+      global.fetch = jest
+        .fn()
+        .mockResolvedValue(mockResponse({ body })) as never;
+      const provider = makeProvider('https://catalog.example.com');
+      await expect(provider.fetchIndex()).rejects.toBeInstanceOf(
+        BadGatewayException,
+      );
+    });
+
+    it('malformed index.yaml (valid YAML, wrong shape) => BadGateway', async () => {
+      // Parses as YAML but fails isCatalogIndex (schemaVersion not a number).
+      const body = streamOf([
+        new TextEncoder().encode(
+          stringifyYaml({ schemaVersion: 'x', bundles: [] }),
        ),
      ]);
      global.fetch = jest
@@ -283,6 +309,36 @@ describe('AiAgentRolesCatalogProvider', () => {
      const provider = makeProvider('https://catalog.example.com');
      await expect(provider.fetchIndex()).rejects.toThrow(/malformed/i);
    });
+
+    it('block-scalar instructions round-trips to the exact multi-line string', async () => {
+      // The whole point of the YAML migration: a long `instructions` prompt is
+      // stored as a literal block scalar (|-) for line-by-line diffs, and must
+      // resolve byte-for-byte to the original multi-line string.
+      const instructions = [
+        'Line one of the prompt.',
+        '',
+        '  Indented bullet that must survive.',
+        'Final line, no trailing newline.',
+      ].join('\n');
+      const yaml = stringifyYaml(
+        {
+          schemaVersion: 1,
+          language: 'en',
+          roles: [{ slug: 'researcher', name: 'Researcher', instructions }],
+        },
+        { lineWidth: 0 },
+      );
+      // Sanity: the fixture really uses a literal block scalar (|, optionally
+      // with an indentation indicator), not a flow/quoted string.
+      expect(yaml).toMatch(/instructions: \|/);
+      const body = streamOf([new TextEncoder().encode(yaml)]);
+      global.fetch = jest
+        .fn()
+        .mockResolvedValue(mockResponse({ body })) as never;
+      const provider = makeProvider('https://catalog.example.com');
+      const bundle = await provider.fetchBundle('research', 'en');
+      expect(bundle.roles[0].instructions).toBe(instructions);
+    });
  });

  describe('path-traversal / SSRF guard (^[a-z0-9-]+$)', () => {
@@ -4,6 +4,7 @@ import {
  Injectable,
  Logger,
 } from '@nestjs/common';
+import { parse as parseYamlDoc } from 'yaml';
 import { EnvironmentService } from '../../../../integrations/environment/environment.service';
 import {
  CatalogBundleFile,
@@ -28,9 +29,11 @@ const MAX_BYTES = 1_000_000;
 * base URL — REMOTE only; local-filesystem sources are no longer supported. The
 * value is baked into the Docker image at build time (set per-branch in CI).
 *
- * The catalog is UNTRUSTED input: every file is JSON-parsed and run through a
- * hand-written type guard before any field is exposed, and every dynamic path
- * segment is validated against SEGMENT_RE up front (path-traversal + SSRF).
+ * The catalog is UNTRUSTED input: every file is YAML-parsed with a SAFE schema
+ * (standard JSON-compatible tags only — no custom `!!` tags / no code execution)
+ * and run through a hand-written type guard before any field is exposed, and
+ * every dynamic path segment is validated against SEGMENT_RE up front
+ * (path-traversal + SSRF).
 */
@Injectable()
 export class AiAgentRolesCatalogProvider {
@@ -38,19 +41,19 @@ export class AiAgentRolesCatalogProvider {

  constructor(private readonly environmentService: EnvironmentService) {}

-  /** Read + validate the top-level index (`index.json`). */
+  /** Read + validate the top-level index (`index.yaml`). */
  async fetchIndex(): Promise<CatalogIndex> {
-    const raw = await this.readRelative('index.json');
-    const parsed = this.parseJson(raw, 'index.json');
+    const raw = await this.readRelative('index.yaml');
+    const parsed = this.parseYaml(raw, 'index.yaml');
    if (!isCatalogIndex(parsed)) {
      throw new BadGatewayException(
-        'Agent roles catalog index is malformed (index.json)',
+        'Agent roles catalog index is malformed (index.yaml)',
      );
    }
    return parsed;
  }

-  /** Read + validate one language file (`bundles/<bundleId>/<language>.json`). */
+  /** Read + validate one language file (`bundles/<bundleId>/<language>.yaml`). */
  async fetchBundle(
    bundleId: string,
    language: string,
@@ -58,9 +61,9 @@ export class AiAgentRolesCatalogProvider {
    // SECURITY: validate BEFORE building any path/URL (path-traversal + SSRF).
    this.assertSegment(bundleId, 'bundleId');
    this.assertSegment(language, 'language');
-    const rel = `bundles/${bundleId}/${language}.json`;
+    const rel = `bundles/${bundleId}/${language}.yaml`;
    const raw = await this.readRelative(rel);
-    const parsed = this.parseJson(raw, rel);
+    const parsed = this.parseYaml(raw, rel);
    if (!isCatalogBundleFile(parsed)) {
      throw new BadGatewayException(
        `Agent roles catalog bundle is malformed (${rel})`,
@@ -76,15 +79,25 @@ export class AiAgentRolesCatalogProvider {
    }
  }

-  /** JSON.parse with a clear BadGateway on malformed content. */
-  private parseJson(raw: string, rel: string): unknown {
+  /**
+   * Safe YAML parse with a clear BadGateway on malformed content. The catalog is
+   * untrusted, so we lean on the `yaml` library's default `core` schema, which
+   * only produces JSON-compatible values (objects/arrays/strings/numbers/
+   * booleans/null) and NEVER constructs arbitrary types or runs code — there is
+   * no `!!js`-style tag handling. `strict: true` rejects unknown custom tags and
+   * duplicate keys instead of silently coercing them, and the alias-expansion
+   * guard (`maxAliasCount`) bounds billion-laughs blow-ups (the 1 MB streaming
+   * cap already limits the input itself). JSON is a YAML subset, so a leftover
+   * `.json`-style body still parses here too.
+   */
+  private parseYaml(raw: string, rel: string): unknown {
    try {
-      return JSON.parse(raw);
+      return parseYamlDoc(raw, { strict: true, maxAliasCount: 100 });
    } catch (err) {
      const reason = shortError(err);
-      this.logger.error(`Agent roles catalog JSON parse failed (${rel}): ${reason}`);
+      this.logger.error(`Agent roles catalog YAML parse failed (${rel}): ${reason}`);
      throw new BadGatewayException(
-        `Agent roles catalog file is not valid JSON (${rel}): ${reason}`,
+        `Agent roles catalog file is not valid YAML (${rel}): ${reason}`,
      );
    }
  }
@@ -1,7 +1,8 @@
 /**
- * Catalog wire shapes. The catalog is curated, untrusted JSON (a GitHub repo or
+ * Catalog wire shapes. The catalog is curated, untrusted YAML (a GitHub repo or
 * a local folder), so every shape is validated by a hand-written type guard in
- * the provider before any field is used — no zod / new deps on the server.
+ * the provider before any field is used — no zod on the server (YAML is parsed
+ * with the `yaml` library's safe, JSON-compatible schema).
 *
 * Localized fields (`name` / `description` at the bundle level) are
 * `Record<language, string>` so one bundle serves many UI languages; per-role
@@ -22,7 +23,7 @@ export interface CatalogRole {
  modelConfig?: Record<string, unknown> | null;
 }

-/** A single language file: `bundles/<id>/<language>.json`. */
+/** A single language file: `bundles/<id>/<language>.yaml`. */
 export interface CatalogBundleFile {
  schemaVersion: number;
  language: string;
@@ -40,7 +41,7 @@ export interface CatalogBundleMeta {
  roles: { slug: string; version: number }[];
 }

-/** Top-level catalog index: `index.json`. */
+/** Top-level catalog index: `index.yaml`. */
 export interface CatalogIndex {
  schemaVersion: number;
  bundles: CatalogBundleMeta[];