From 46972a31b6b81203bb497b5411f15d198a5adfaf Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 14:29:55 +0200 Subject: [PATCH 1/8] fix(deps): bump project-engine + user-manager clients to 1.3.0 Aligns the typed Semrush clients with the published Counterfact mock Docker images (only 1.3.0 is published for both), so the serenity IT exercises the SAME contract version we ship. The bump also resolves the `init_status` typed-path that was missing from the prior 1.2.0/1.1.0 client surface. Both are minor, backward-compatible bumps within 1.x. Co-Authored-By: Claude Opus 4.8 (1M context) --- package-lock.json | 16 ++++++++-------- package.json | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index f73c53a5b..fa73bdc2b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,13 +28,13 @@ "@adobe/spacecat-shared-http-utils": "1.31.0", "@adobe/spacecat-shared-ims-client": "1.12.7", "@adobe/spacecat-shared-launchdarkly-client": "1.3.0", - "@adobe/spacecat-shared-project-engine-client": "1.2.0", + "@adobe/spacecat-shared-project-engine-client": "1.3.0", "@adobe/spacecat-shared-rum-api-client": "2.40.13", "@adobe/spacecat-shared-scrape-client": "2.6.3", "@adobe/spacecat-shared-slack-client": "1.6.7", "@adobe/spacecat-shared-tier-client": "1.5.1", "@adobe/spacecat-shared-tokowaka-client": "1.19.0", - "@adobe/spacecat-shared-user-manager-client": "1.1.0", + "@adobe/spacecat-shared-user-manager-client": "1.3.0", "@adobe/spacecat-shared-utils": "1.122.1", "@adobe/spacecat-shared-vault-secrets": "1.3.5", "@aws-sdk/client-s3": "3.1045.0", @@ -6925,9 +6925,9 @@ } }, "node_modules/@adobe/spacecat-shared-project-engine-client": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-project-engine-client/-/spacecat-shared-project-engine-client-1.2.0.tgz", - "integrity": "sha512-WOcTN//+8wVTGvtG7w7b38bftUr+nKjPkDAHbwNpZ/bdIO65V2GQKXXXBAE8cCOkzi26mxedFkIaP/Qoqu9BCA==", + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-project-engine-client/-/spacecat-shared-project-engine-client-1.3.0.tgz", + "integrity": "sha512-ZOEbfH4vaMd/1kz2bM5WF4thsoOJ3i6S+oIgnFmwKllDfxQH8PNnVAfVhWjxCJoYNcva8y4/87sCmnwk2CDq3Q==", "license": "Apache-2.0", "dependencies": { "openapi-fetch": "0.17.0" @@ -10278,9 +10278,9 @@ } }, "node_modules/@adobe/spacecat-shared-user-manager-client": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-user-manager-client/-/spacecat-shared-user-manager-client-1.1.0.tgz", - "integrity": "sha512-lNCJDWMUy8/JMlZV9KTBB5uMsMZs494LyJLYBmCVplrquEO+pOizyrh6obPVsofhTS67UAfmvSv/WNRbFDNN9A==", + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-user-manager-client/-/spacecat-shared-user-manager-client-1.3.0.tgz", + "integrity": "sha512-EQGmOMwf8BqsQIoc/8zAcQbUo8fc5eeZLSf+154yvesTT9o4/t1MnXtav9dpsV/JWE00jXC0ckPx6c+TY9dkPA==", "license": "Apache-2.0", "dependencies": { "openapi-fetch": "0.17.0" diff --git a/package.json b/package.json index c56a83f8e..48c494507 100644 --- a/package.json +++ b/package.json @@ -93,13 +93,13 @@ "@adobe/spacecat-shared-http-utils": "1.31.0", "@adobe/spacecat-shared-ims-client": "1.12.7", "@adobe/spacecat-shared-launchdarkly-client": "1.3.0", - "@adobe/spacecat-shared-project-engine-client": "1.2.0", + "@adobe/spacecat-shared-project-engine-client": "1.3.0", "@adobe/spacecat-shared-rum-api-client": "2.40.13", "@adobe/spacecat-shared-scrape-client": "2.6.3", "@adobe/spacecat-shared-slack-client": "1.6.7", "@adobe/spacecat-shared-tier-client": "1.5.1", "@adobe/spacecat-shared-tokowaka-client": "1.19.0", - "@adobe/spacecat-shared-user-manager-client": "1.1.0", + "@adobe/spacecat-shared-user-manager-client": "1.3.0", "@adobe/spacecat-shared-utils": "1.122.1", "@adobe/spacecat-shared-vault-secrets": "1.3.5", "@aws-sdk/client-s3": "3.1045.0", From ceee5e2eef98458c708c50e0e9fac62015b5caf5 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 14:30:14 +0200 Subject: [PATCH 2/8] refactor(serenity): split Semrush User Manager base URL (#2656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add usersBaseUrl(env) reading SEMRUSH_USERS_BASE_URL, falling back to SEMRUSH_PROJECTS_BASE_URL when unset, and route the typed User Manager client through it. Factors the trim/https/origin-normalize validation into a shared normalizeBaseUrl() reused by both resolvers (https kept). Lets the User Manager calls point at a separate (mock) host independently of Project Engine — no path-routing reverse proxy — which unblocks local + automated E2E of the serenity sub-workspace flows. Prod/stage/dev Vault config (single host today) keeps working untouched via the fallback. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/support/serenity/rest-transport.js | 90 +++++++++++++++----- test/support/serenity/rest-transport.test.js | 52 +++++++++++ 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/src/support/serenity/rest-transport.js b/src/support/serenity/rest-transport.js index abfd6940d..795ce0932 100644 --- a/src/support/serenity/rest-transport.js +++ b/src/support/serenity/rest-transport.js @@ -67,53 +67,91 @@ export function redactUpstreamMessage(e) { } /** - * Resolves and validates the upstream base URL. The URL is required and must - * arrive via `env.SEMRUSH_PROJECTS_BASE_URL` — sourced from Vault - * (`dx_mysticat//api-service`) and injected through AWS Secrets Manager. - * No source default: the upstream host is operational config that must be - * settable per-environment without a code change. + * Validates a raw base-URL value and returns its canonical `protocol//host` + * origin — never the raw value. A misconfigured value like + * `https://host/path-prefix` or `https://user:pass@host` would otherwise + * silently bleed path/userinfo into every outbound request (and the userinfo + * form would leak credentials in each fetch's `Authorization`-adjacent + * metadata). Always returning the parsed origin closes both classes of + * injection. The https requirement is kept for both resolvers. * - * Returns the canonical `protocol//host` origin — never the raw value. A - * misconfigured value like `https://host/path-prefix` or - * `https://user:pass@host` would otherwise silently bleed path/userinfo into - * every outbound request (and the userinfo form would leak credentials in - * each fetch's `Authorization`-adjacent metadata). Always returning the - * parsed origin closes both classes of injection. + * `varName` names the env var in every error message so a misconfiguration + * points the operator at the exact key to fix. * * Failure mapping (controller `mapError`): * - Missing/invalid/non-https → throws ErrorWithStatusCode(503, * 'configurationError'): operational failure, not a runtime bug. + * + * @param {string | undefined} raw - The raw env value. + * @param {string} varName - The env var name, used in error messages. + * @returns {string} The canonical `protocol//host` origin. */ -function baseUrl(env) { - const raw = typeof env?.SEMRUSH_PROJECTS_BASE_URL === 'string' - ? env.SEMRUSH_PROJECTS_BASE_URL.trim() - : env?.SEMRUSH_PROJECTS_BASE_URL; - if (!hasText(raw)) { +function normalizeBaseUrl(raw, varName) { + // Default a non-string (incl. undefined) to '' so the value is always a + // string for hasText/replace below — hasText is not a TS type guard, so the + // narrowing has to be structural (see this dir's CLAUDE.md). + const trimmed = typeof raw === 'string' ? raw.trim() : ''; + if (!hasText(trimmed)) { throw new ErrorWithStatusCode( - 'SEMRUSH_PROJECTS_BASE_URL is not set. Configure it via Vault ' + `${varName} is not set. Configure it via Vault ` + '(dx_mysticat//api-service) or .env for local dev.', 503, ); } - const candidate = raw.replace(/\/$/, ''); + const candidate = trimmed.replace(/\/$/, ''); let parsed; try { parsed = new URL(candidate); } catch { throw new ErrorWithStatusCode( - `SEMRUSH_PROJECTS_BASE_URL is not a valid URL: ${candidate}`, + `${varName} is not a valid URL: ${candidate}`, 503, ); } if (parsed.protocol !== 'https:') { throw new ErrorWithStatusCode( - `SEMRUSH_PROJECTS_BASE_URL must use https (got ${parsed.protocol})`, + `${varName} must use https (got ${parsed.protocol})`, 503, ); } return `${parsed.protocol}//${parsed.host}`; } +/** + * Resolves the Project Engine gateway origin. Required; arrives via + * `env.SEMRUSH_PROJECTS_BASE_URL` — sourced from Vault + * (`dx_mysticat//api-service`) and injected through AWS Secrets Manager. + * No source default: the upstream host is operational config that must be + * settable per-environment without a code change. + * + * @param {object} env + * @returns {string} canonical `protocol//host` origin + */ +function baseUrl(env) { + return normalizeBaseUrl(env?.SEMRUSH_PROJECTS_BASE_URL, 'SEMRUSH_PROJECTS_BASE_URL'); +} + +/** + * Resolves the User Manager gateway origin. Reads `env.SEMRUSH_USERS_BASE_URL`, + * **falling back to `SEMRUSH_PROJECTS_BASE_URL` when unset** — Project Engine + * and User Manager are distinct Semrush services that share a single host in + * every deployed environment today, so the fallback keeps prod/stage/dev Vault + * config working untouched. Decoupling lets local + E2E setups point the User + * Manager calls at a SEPARATE (mock) host without a path-routing reverse proxy + * (LLMO / api-service#2656). The error message names whichever var was the + * effective source so a misconfiguration is unambiguous. + * + * @param {object} env + * @returns {string} canonical `protocol//host` origin + */ +function usersBaseUrl(env) { + const explicit = hasText(env?.SEMRUSH_USERS_BASE_URL); + return normalizeBaseUrl( + explicit ? env.SEMRUSH_USERS_BASE_URL : env?.SEMRUSH_PROJECTS_BASE_URL, + explicit ? 'SEMRUSH_USERS_BASE_URL' : 'SEMRUSH_PROJECTS_BASE_URL', + ); +} + /** * Wraps global fetch with the transport's 15s ceiling and the * `Accept: application/json` header sent on every call — neither of which the @@ -183,11 +221,17 @@ function unwrap(method, result) { * separate user-manager gateway. * * @param {object} args - * @param {object} args.env - Environment (reads SEMRUSH_PROJECTS_BASE_URL override). + * @param {object} args.env - Environment (reads SEMRUSH_PROJECTS_BASE_URL and, + * for the User Manager gateway, SEMRUSH_USERS_BASE_URL — falling back to the + * projects host when the latter is unset). * @param {string} args.imsToken - IMS user bearer token (without 'Bearer ' prefix). */ export function createSerenityTransport({ env, imsToken }) { const root = baseUrl(env); + // User Manager has its OWN origin (SEMRUSH_USERS_BASE_URL), falling back to + // `root` when unset — see usersBaseUrl(). Lets the sub-workspace lifecycle + // calls hit a separate (mock) host independently of Project Engine. + const usersRoot = usersBaseUrl(env); // Fail-closed guard for the destructive workspace delete. Deleting a // sub-workspace must be IMPOSSIBLE in every deployed environment @@ -225,8 +269,10 @@ export function createSerenityTransport({ env, imsToken }) { // Typed User Manager client over the sub-workspace lifecycle gateway. Same // shape as the project client; appends its own '/enterprise/users/api' prefix. + // Uses `usersRoot` (SEMRUSH_USERS_BASE_URL, or `root` by fallback) so the + // lifecycle gateway can be a separate host from Project Engine. const users = createSerenityUserManagerApiClient({ - baseUrl: root, + baseUrl: usersRoot, authToken, maxRetries: 0, fetch: createTimeoutFetch(DEFAULT_TIMEOUT_MS), diff --git a/test/support/serenity/rest-transport.test.js b/test/support/serenity/rest-transport.test.js index 641b60507..889730a9a 100644 --- a/test/support/serenity/rest-transport.test.js +++ b/test/support/serenity/rest-transport.test.js @@ -215,6 +215,58 @@ describe('Semrush REST transport', () => { } }); + // ── User Manager base-URL split (api-service#2656) ── + // The User Manager gateway resolves its own origin from SEMRUSH_USERS_BASE_URL, + // falling back to SEMRUSH_PROJECTS_BASE_URL when unset. getWorkspaceStatus is a + // User-Manager-gateway method; publishProject is a Project-Engine method. + it('routes User Manager calls at SEMRUSH_USERS_BASE_URL when set, Project Engine at SEMRUSH_PROJECTS_BASE_URL', async () => { + fetchStub.resolves(fetchOk(null)); + const transport = createSerenityTransport({ + env: { + SEMRUSH_PROJECTS_BASE_URL: 'https://projects.semrush.test', + SEMRUSH_USERS_BASE_URL: 'https://users.semrush.test', + }, + imsToken: IMS, + }); + + await transport.getWorkspaceStatus(WORKSPACE_ID); + expect((await callOf(fetchStub)).url) + .to.match(/^https:\/\/users\.semrush\.test\/enterprise\/users\/api\//); + + await transport.publishProject(WORKSPACE_ID, PROJECT_ID); + expect((await callOf(fetchStub, 1)).url) + .to.match(/^https:\/\/projects\.semrush\.test\/enterprise\/projects\/api\//); + }); + + it('falls back to SEMRUSH_PROJECTS_BASE_URL for User Manager calls when SEMRUSH_USERS_BASE_URL is unset', async () => { + fetchStub.resolves(fetchOk(null)); + const transport = createSerenityTransport({ + env: { SEMRUSH_PROJECTS_BASE_URL: 'https://shared.semrush.test' }, + imsToken: IMS, + }); + + await transport.getWorkspaceStatus(WORKSPACE_ID); + + expect((await callOf(fetchStub)).url) + .to.match(/^https:\/\/shared\.semrush\.test\/enterprise\/users\/api\//); + }); + + it('rejects a non-https SEMRUSH_USERS_BASE_URL naming the USERS var (503)', () => { + try { + createSerenityTransport({ + env: { + SEMRUSH_PROJECTS_BASE_URL: 'https://projects.semrush.test', + SEMRUSH_USERS_BASE_URL: 'http://attacker.example/', + }, + imsToken: IMS, + }); + expect.fail('expected createSerenityTransport to throw'); + } catch (e) { + expect(e.message).to.match(/SEMRUSH_USERS_BASE_URL must use https/); + expect(e.status).to.equal(503); + } + }); + it('encodes path segments so reserved chars stay inside the segment', async () => { fetchStub.resolves(fetchOk({ items: [] })); const transport = createSerenityTransport({ env: TEST_ENV, imsToken: IMS }); From 18927907343a0eb6c2fb648ae835861f3f624d45 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 14:30:32 +0200 Subject: [PATCH 3/8] test(serenity): API-level e2e against the Semrush vendor mocks Drive the /serenity/* controller end to end through the public GHCR Counterfact mocks (project-engine + user-manager) in the postgres IT harness, replacing the prior 400/401-only contract suite. - Boot both mocks via the IT docker-compose (own HTTPS port each); the image tag is the INSTALLED client version (setup.js exports SERENITY_*_MOCK_TAG) so the mock can never drift from the client we ship. env.js points SEMRUSH_PROJECTS_BASE_URL / SEMRUSH_USERS_BASE_URL at them with NODE_TLS_REJECT_UNAUTHORIZED=0 for their self-signed certs. - requireImsBearer honours a test-only SERENITY_ALLOW_NON_IMS_AUTH flag so the harness JWT reaches the handlers; sound only vs the mocks, which ignore the forwarded bearer. No deployed environment sets the flag (mirrors SERENITY_ALLOW_WORKSPACE_DELETE). - setup.js waits on the mock control routes and exposes resetSemrushMocks; the factory seeds via resetPostgres. - First increment: route-gate 400s, org-level model/language catalog live via the Project Engine mock, and brand-resolution after the relaxed auth. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/controllers/serenity.js | 14 +- test/controllers/serenity.test.js | 22 ++++ test/it/env.js | 16 +++ test/it/postgres/docker-compose.yml | 45 +++++++ test/it/postgres/serenity.test.js | 3 +- test/it/postgres/setup.js | 91 ++++++++++++- test/it/shared/tests/serenity.js | 198 ++++++++++------------------ 7 files changed, 255 insertions(+), 134 deletions(-) diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index 0c7315ec1..52b4fd248 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -180,10 +180,22 @@ function mapError(e, log) { * missing OR if the caller authenticated by some other mechanism. The * upstream gateway only understands IMS user tokens; we refuse to forward * anything else. + * + * Test-only escape hatch: when `SERENITY_ALLOW_NON_IMS_AUTH === 'true'` the + * IMS-type check is skipped so an authenticated NON-IMS caller (e.g. the + * locally-signed JWT the integration-test harness mints) can reach the + * handlers. This is sound ONLY against the Semrush vendor MOCKS, which do not + * validate the forwarded bearer — the token's value never matters, only that + * an authenticated identity is present. Mirrors `SERENITY_ALLOW_WORKSPACE_DELETE` + * in rest-transport.js: an explicit opt-in flag that NO deployed environment + * sets (it is never written to Vault `dx_mysticat//api-service`); it is + * for local + automated E2E only. The Authorization-header requirement still + * holds — a bearer must be present to forward upstream. */ function requireImsBearer(ctx) { const authInfo = ctx?.attributes?.authInfo; - if (authInfo?.getType && authInfo.getType() !== 'ims') { + const allowNonIms = ctx?.env?.SERENITY_ALLOW_NON_IMS_AUTH === 'true'; + if (!allowNonIms && authInfo?.getType && authInfo.getType() !== 'ims') { throw new ErrorWithStatusCode( 'Serenity proxy requires IMS authentication', 401, diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index ab79ba097..cc273d425 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -258,6 +258,28 @@ describe('SerenityController', () => { expect(response.status).to.equal(401); }); + // Test-only escape hatch (SERENITY_ALLOW_NON_IMS_AUTH). The integration-test + // harness mints a non-IMS (JWT) token; with the flag set, the IMS-type gate + // is skipped so the handler runs (the Semrush mock ignores the forwarded + // bearer). The Authorization header is still required (asserted below). + it('lets a non-IMS caller through when SERENITY_ALLOW_NON_IMS_AUTH is set (reaches the handler, not 401)', async () => { + handlers.handleListPrompts.resolves({ items: [], total: 0 }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const ctx = fakeContext({ authType: 'jwt', env: { SERENITY_ALLOW_NON_IMS_AUTH: 'true' } }); + const response = await controller.listPrompts(ctx); + expect(response.status).to.equal(200); + expect(handlers.handleListPrompts).to.have.been.calledOnce; + }); + + it('still 401s a non-IMS caller with the flag set but NO Authorization header', async () => { + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const ctx = fakeContext({ + authType: 'jwt', bearer: null, env: { SERENITY_ALLOW_NON_IMS_AUTH: 'true' }, + }); + const response = await controller.listPrompts(ctx); + expect(response.status).to.equal(401); + }); + it('400s when :brandId is not a UUID (the new guard)', async () => { const controller = SerenityController({ env: {} }, fakeLog(), {}); const ctx = fakeContext({ brandId: 'adobe-brand-name' }); diff --git a/test/it/env.js b/test/it/env.js index a963f7fe6..66f8f3e1e 100644 --- a/test/it/env.js +++ b/test/it/env.js @@ -98,5 +98,21 @@ export function buildEnv(publicKeyB64) { POSTGREST_URL: `http://localhost:${process.env.IT_POSTGREST_PORT || '3300'}`, POSTGREST_SCHEMA: 'public', POSTGREST_API_KEY: POSTGREST_WRITER_JWT, + + // ── Serenity E2E: Semrush vendor mocks ────────────────────────────────── + // Point the two serenity transport gateways at the mock containers. The + // User Manager origin is split out via SEMRUSH_USERS_BASE_URL (api-service#2656) + // so the two mocks need no path-routing reverse proxy. Both serve self-signed + // HTTPS, so the dev server trusts them via NODE_TLS_REJECT_UNAUTHORIZED=0 + // (scoped to this IT process — never a deployed setting). + SEMRUSH_PROJECTS_BASE_URL: `https://localhost:${process.env.IT_PE_MOCK_PORT || '8443'}`, + SEMRUSH_USERS_BASE_URL: `https://localhost:${process.env.IT_UM_MOCK_PORT || '8444'}`, + NODE_TLS_REJECT_UNAUTHORIZED: '0', + // Test-only escape hatch: accept the harness's non-IMS JWT on /serenity/*. + // Sound only against the mocks, which ignore the forwarded bearer. No + // deployed environment sets this (it is never written to Vault). + SERENITY_ALLOW_NON_IMS_AUTH: 'true', + // Lets the net-zero cleanup delete a sub-workspace it created in the mock. + SERENITY_ALLOW_WORKSPACE_DELETE: 'true', }; } diff --git a/test/it/postgres/docker-compose.yml b/test/it/postgres/docker-compose.yml index 5eaedad9e..f4d031bbc 100644 --- a/test/it/postgres/docker-compose.yml +++ b/test/it/postgres/docker-compose.yml @@ -61,3 +61,48 @@ services: ports: - "${IT_POSTGREST_PORT:-3300}:3000" - "${IT_POSTGREST_ADMIN_PORT:-3301}:3001" + + # ── Semrush vendor mocks (serenity E2E) ────────────────────────────────── + # Stateful Counterfact mocks of the Semrush Project Engine and User Manager + # gateways, published public to GHCR from adobe/spacecat-shared. Each serves + # ONLY its own path prefix over self-signed HTTPS on internal :8443 (Caddy); + # the dev server reaches them via localhost with NODE_TLS_REJECT_UNAUTHORIZED=0 + # and the SEMRUSH_PROJECTS_BASE_URL / SEMRUSH_USERS_BASE_URL split (api-service#2656). + # Public images → no registry credentials needed in CI. Control routes + # (//__reset, /__seed, /__dump) are unauthenticated; reset between tests. + # + # Image tag == the installed client dependency version. The mock is published + # from the SAME package as the typed client (@adobe/spacecat-shared-*-client), + # so testing against a different mock version than the client we ship would be + # an unsound contract test. setup.js reads the installed client version and + # exports SERENITY_PE_MOCK_TAG / SERENITY_UM_MOCK_TAG; the `:-` default keeps a + # manual `docker compose up` working. If a client is bumped to a version whose + # mock image is not yet published, the pull fails loudly — by design, it forces + # the mock to be released in lockstep. + project-engine-mock: + image: ghcr.io/adobe/spacecat-shared-project-engine-client-mock:${SERENITY_PE_MOCK_TAG:-1.3.0} + container_name: spacecat-it-pe-mock + environment: + MOCK_SEED: workspace-with-data + ports: + - "${IT_PE_MOCK_PORT:-8443}:8443" + healthcheck: + test: ["CMD", "curl", "-ksf", "https://localhost:8443/enterprise/projects/api/__dump"] + interval: 5s + timeout: 3s + retries: 20 + start_period: 20s + + user-manager-mock: + image: ghcr.io/adobe/spacecat-shared-user-manager-client-mock:${SERENITY_UM_MOCK_TAG:-1.3.0} + container_name: spacecat-it-um-mock + environment: + MOCK_SEED: parent-with-child + ports: + - "${IT_UM_MOCK_PORT:-8444}:8443" + healthcheck: + test: ["CMD", "curl", "-ksf", "https://localhost:8443/enterprise/users/api/__dump"] + interval: 5s + timeout: 3s + retries: 20 + start_period: 20s diff --git a/test/it/postgres/serenity.test.js b/test/it/postgres/serenity.test.js index 7c465c275..786074a4e 100644 --- a/test/it/postgres/serenity.test.js +++ b/test/it/postgres/serenity.test.js @@ -11,6 +11,7 @@ */ import { ctx } from './harness.js'; +import { resetPostgres } from './seed.js'; import serenityTests from '../shared/tests/serenity.js'; -serenityTests(() => ctx.httpClient); +serenityTests(() => ctx.httpClient, resetPostgres); diff --git a/test/it/postgres/setup.js b/test/it/postgres/setup.js index 2fefff1bd..f5c509733 100644 --- a/test/it/postgres/setup.js +++ b/test/it/postgres/setup.js @@ -12,17 +12,44 @@ /* eslint-disable no-await-in-loop, no-underscore-dangle, max-statements-per-line */ import { execSync } from 'child_process'; +import { readFileSync } from 'fs'; import { fileURLToPath } from 'url'; import path from 'path'; import { S3Client, CreateBucketCommand, HeadBucketCommand } from '@aws-sdk/client-s3'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const COMPOSE_FILE = path.join(__dirname, 'docker-compose.yml'); +const REPO_ROOT = path.join(__dirname, '..', '..', '..'); + +/** + * Reads the INSTALLED version of a dependency from its package.json. The mock + * Docker image is published from the same package as the typed client, so the + * mock tag must equal the client version we actually ship — otherwise the IT + * runs against a different contract than production. Read via fs (not require) + * because the client packages restrict the `./package.json` subpath in exports. + * + * @param {string} pkg - npm package name + * @returns {string} the installed semver + */ +function installedVersion(pkg) { + const pkgJson = path.join(REPO_ROOT, 'node_modules', pkg, 'package.json'); + return JSON.parse(readFileSync(pkgJson, 'utf8')).version; +} const POSTGREST_PORT = process.env.IT_POSTGREST_PORT || '3300'; const POSTGREST_URL = `http://localhost:${POSTGREST_PORT}`; const MINIO_PORT = process.env.IT_MINIO_PORT || '9100'; const MINIO_HEALTH_URL = `http://localhost:${MINIO_PORT}/minio/health/live`; +// Semrush vendor mocks (serenity E2E). Self-signed HTTPS; the readiness probe +// and the reset helper both talk to the unauthenticated control routes, so they +// run with TLS verification disabled regardless of the dev server's setting. +const PE_MOCK_PORT = process.env.IT_PE_MOCK_PORT || '8443'; +const UM_MOCK_PORT = process.env.IT_UM_MOCK_PORT || '8444'; +const PE_MOCK_BASE = `https://localhost:${PE_MOCK_PORT}/enterprise/projects/api`; +const UM_MOCK_BASE = `https://localhost:${UM_MOCK_PORT}/enterprise/users/api`; +const MOCK_DUMP_PATHS = [`${PE_MOCK_BASE}/__dump`, `${UM_MOCK_BASE}/__dump`]; +const MOCK_RESET_PATHS = [`${PE_MOCK_BASE}/__reset`, `${UM_MOCK_BASE}/__reset`]; + /** * Polls the PostgREST admin endpoint until it responds. * @@ -67,6 +94,60 @@ async function waitForMinio(maxAttempts = 60, intervalMs = 500) { throw new Error(`MinIO did not become ready within ${maxAttempts * intervalMs}ms`); } +/** + * Polls both Semrush mock control endpoints until they respond over HTTPS. + * Uses a per-request fetch with TLS verification disabled (self-signed cert). + * + * @param {number} maxAttempts - Maximum poll attempts + * @param {number} intervalMs - Delay between attempts + */ +async function waitForSemrushMocks(maxAttempts = 60, intervalMs = 1000) { + const prev = process.env.NODE_TLS_REJECT_UNAUTHORIZED; + process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; + try { + for (let i = 0; i < maxAttempts; i += 1) { + try { + const results = await Promise.all( + MOCK_DUMP_PATHS.map((url) => fetch(url).then((r) => r.ok).catch(() => false)), + ); + if (results.every(Boolean)) { + return; + } + } catch { + // Not ready yet + } + await new Promise((resolve) => { setTimeout(resolve, intervalMs); }); + } + throw new Error(`Semrush mocks did not become ready within ${maxAttempts * intervalMs}ms`); + } finally { + if (prev === undefined) { + delete process.env.NODE_TLS_REJECT_UNAUTHORIZED; + } else { + process.env.NODE_TLS_REJECT_UNAUTHORIZED = prev; + } + } +} + +/** + * Resets both Semrush mocks to their boot seed. Call between test cases that + * mutate mock state (activate/create/delete) so each starts from a known store. + */ +export async function resetSemrushMocks() { + const prev = process.env.NODE_TLS_REJECT_UNAUTHORIZED; + process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; + try { + await Promise.all( + MOCK_RESET_PATHS.map((url) => fetch(url, { method: 'POST' }).catch(() => {})), + ); + } finally { + if (prev === undefined) { + delete process.env.NODE_TLS_REJECT_UNAUTHORIZED; + } else { + process.env.NODE_TLS_REJECT_UNAUTHORIZED = prev; + } + } +} + /** * Creates the MinIO bucket used by IT tests if it does not already exist. * MinIO is S3-compatible so `NoSuchBucket` errors are replaced by `HeadBucket 404`. @@ -92,15 +173,23 @@ async function ensureMinIoBucket() { * @returns {Promise} The PostgREST base URL */ export async function startPostgres() { + // Pin each Semrush mock image to the version of the typed client we actually + // depend on (the mock is published from that same package). Drift here would + // silently test a different contract than production ships. A bumped client + // whose mock image is not yet published makes the pull fail loudly — by design. + process.env.SERENITY_PE_MOCK_TAG = installedVersion('@adobe/spacecat-shared-project-engine-client'); + process.env.SERENITY_UM_MOCK_TAG = installedVersion('@adobe/spacecat-shared-user-manager-client'); + execSync( `docker compose -f "${COMPOSE_FILE}" up -d`, { stdio: 'inherit', timeout: 120_000 }, ); - // Wait for PostgREST and MinIO to become ready in parallel + // Wait for PostgREST, MinIO and the Semrush mocks to become ready in parallel await Promise.all([ waitForPostgREST(), waitForMinio(), + waitForSemrushMocks(), ]); await ensureMinIoBucket(); diff --git a/test/it/shared/tests/serenity.js b/test/it/shared/tests/serenity.js index 261f698c2..3a92a0fc5 100644 --- a/test/it/shared/tests/serenity.js +++ b/test/it/shared/tests/serenity.js @@ -14,155 +14,91 @@ import { expect } from 'chai'; import { ORG_1_ID, BRAND_1_ID } from '../seed-ids.js'; /** - * Integration tests for the /serenity/* surface (LLMO-5190). + * End-to-end tests for the /serenity/* surface (LLMO-5190), driven against the + * Semrush vendor MOCKS (Counterfact images from adobe/spacecat-shared, started + * by the IT docker-compose). Two things make these reachable where the prior + * IT suite could only assert 400/401: * - * Scope and limitation - * ──────────────────── - * The serenity controller enforces IMS-only authentication - * (`requireImsBearer`) on every handler dispatch. The shared IT harness mints - * JWT tokens via `test/it/shared/auth.js` — those are not IMS-typed, so any - * authenticated GET against `/serenity/*` deterministically returns 401 from - * the controller before reaching the handler. Until the harness grows the - * ability to mint IMS-shaped tokens (and the local dev server is configured - * to trust them), the IT-testable surface is restricted to: + * 1. Auth: the harness mints a NON-IMS (local JWT) token, which the serenity + * controller's `requireImsBearer` normally rejects (it forwards only IMS + * tokens upstream). The IT env sets `SERENITY_ALLOW_NON_IMS_AUTH=true`, + * which skips the IMS-type gate — sound ONLY because the Semrush mock does + * not validate the forwarded bearer (the token value never matters). No + * deployed environment sets that flag. + * 2. Vendor: `SEMRUSH_PROJECTS_BASE_URL` / `SEMRUSH_USERS_BASE_URL` point at + * the two mock containers (api-service#2656 splits the User Manager origin + * so no path-routing proxy is needed); `NODE_TLS_REJECT_UNAUTHORIZED=0` + * trusts their self-signed certs. * - * 1. Route-gate validation (`src/index.js:349-352`) which fires BEFORE auth: - * non-UUID spaceCatId / brandId → 400 with a deterministic message. - * 2. The controller's 401 contract itself: JWT-authenticated requests must - * not be silently accepted by the IMS-required serenity proxy. - * - * The end-to-end "list/create/delete market" and "required-filter 400 on - * prompts/tags/models" paths are covered by the unit suites - * (`test/support/serenity/handlers/*.test.js` — 100% line + branch coverage) - * and the OpenAPI contract suite (`test/openapi-contract/serenity-api.test.js`). - * The `tagIds` multi-value query param added in feat/serenity-tag-filter is - * verified at IT level by the test below (repeated params reach extractQuery - * without a 500, then 401 from requireImsBearer as expected). - * The remaining IT gap is structural (auth-token shape) and is filed for - * follow-up — the workaround alternatives (stubbing requireImsBearer at IT - * time; injecting a fake IMS token-mint) would either ship test-only code - * into production paths or duplicate the auth harness in a way that drifts - * independently. + * This increment covers the route gate, the IMS-only relaxation, and the + * brand-INDEPENDENT catalog reads that flow all the way to the Project Engine + * mock. The sub-workspace lifecycle (activate/deactivate, market create/delete) + * mutates mock state and is the next increment — `resetSemrushMocks()` in + * setup.js is wired for it. */ -export default function serenityTests(getHttpClient) { - describe('Serenity API — route-gate + auth contract (LLMO-5190)', () => { - it('400s on non-UUID spaceCatId (route gate, before auth)', async () => { - const http = getHttpClient(); - const res = await http.admin.get(`/v2/orgs/not-a-uuid/brands/${BRAND_1_ID}/serenity/markets`); - expect(res.status).to.equal(400); - // The 400 message is owned by src/index.js — we assert the substring - // that downstream callers grep for in their own error mapping. - expect(res.body.message || res.body).to.match(/Organization Id.*invalid/i); - }); - - it('400s on non-UUID brandId (route gate, before auth)', async () => { - const http = getHttpClient(); - const res = await http.admin.get(`/v2/orgs/${ORG_1_ID}/brands/not-a-uuid/serenity/markets`); - expect(res.status).to.equal(400); - }); - - // Locks the contract: the serenity proxy refuses anything that isn't an - // IMS-typed token. JWT-authenticated callers (which the harness mints by - // default) get a 401 before the handler ever runs — this prevents - // accidentally widening the proxy's accepted auth shape later. - it('401s when the caller is authenticated but not via IMS', async () => { - const http = getHttpClient(); - const res = await http.admin.get(`/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/markets`); - expect(res.status).to.equal(401); - }); - - it('401s on prompts endpoint with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.get( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/prompts?geoTargetId=2840&languageCode=en`, - ); - expect(res.status).to.equal(401); - }); - - // Verifies that repeated tagIds query params (the new filter feature) are - // accepted by extractQuery / parsedQuery without a 500 and that auth still - // fires — i.e. the multi-value param handling does not crash the controller - // before it reaches requireImsBearer. - it('401s on prompts endpoint with tagIds params (multi-value param does not crash controller)', async () => { - const http = getHttpClient(); - const res = await http.admin.get( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/prompts?geoTargetId=2840&languageCode=en&tagIds=uuid-1&tagIds=uuid-2`, - ); - expect(res.status).to.equal(401); - }); - - it('401s on tags endpoint with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.get( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/tags?geoTargetId=2840&languageCode=en`, - ); - expect(res.status).to.equal(401); - }); +export default function serenityTests(getHttpClient, resetData) { + // Seed the baseline org/brand rows the catalog + brand-resolution tests read. + // (The route-gate cases fire before any DB access, but the org-level reads + // need ORG_1 present.) Mirrors every other postgres factory. + before(() => resetData()); - it('401s on models endpoint with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.get( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/models?geoTargetId=2840&languageCode=en`, + describe('Serenity API — route gate (fires before auth)', () => { + it('400s on non-UUID spaceCatId', async () => { + const res = await getHttpClient().admin.get( + `/v2/orgs/not-a-uuid/brands/${BRAND_1_ID}/serenity/markets`, ); - expect(res.status).to.equal(401); - }); - - it('401s on DELETE market with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.delete( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/markets/2840/en`, - ); - expect(res.status).to.equal(401); + expect(res.status).to.equal(400); + expect(res.body.message || res.body).to.match(/Organization Id.*invalid/i); }); - it('401s on GET market detail with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.get( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/markets/2840/en`, + it('400s on non-UUID brandId', async () => { + const res = await getHttpClient().admin.get( + `/v2/orgs/${ORG_1_ID}/brands/not-a-uuid/serenity/markets`, ); - expect(res.status).to.equal(401); + expect(res.status).to.equal(400); }); - // ── Dual-mode (sub-workspace) routes — same route-gate + IMS-only contract ── - // The subworkspace dispatch + activate/deactivate handler behaviour needs - // an IMS token AND the live Semrush gateway, so (like the rest of this suite) - // it is covered by the unit + contract suites and the live through-api e2e. - // Here we only lock that the NEW routes are registered and enforce the same - // pre-handler contract as the rest of the surface. - it('401s on PUT models with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.put( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/models`, - { geoTargetId: 2840, languageCode: 'en', modelIds: [] }, + it('400s on non-UUID brandId for activate', async () => { + const res = await getHttpClient().admin.post( + `/v2/orgs/${ORG_1_ID}/brands/not-a-uuid/serenity/activate`, + { brandDomain: 'example.com', brandNames: ['Example'], markets: [{ market: 'US', languageCode: 'en' }] }, ); - expect(res.status).to.equal(401); + expect(res.status).to.equal(400); }); + }); - it('401s on POST activate with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.post( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/activate`, - { brandDomain: 'example.com', brandNames: ['Example'], markets: [{ market: 'US', languageCode: 'en' }] }, - ); - expect(res.status).to.equal(401); + describe('Serenity API — org-level catalog (live via Project Engine mock)', () => { + // GET /v2/orgs/:org/serenity/models is brand/workspace-INDEPENDENT: it + // authorizes at the org level and reads the global `GET /v1/ai_models` + // catalog from the Project Engine mock. A 200 here proves the full chain: + // relaxed auth → org access → typed transport → HTTPS to the mock → parse. + it('GET /serenity/models returns 200 with the global AI model catalog', async () => { + const res = await getHttpClient().admin.get(`/v2/orgs/${ORG_1_ID}/serenity/models`); + expect(res.status).to.equal(200); + // The global catalog comes back as { items: [...] }; the mock's + // workspace-with-data seed ships a non-empty model list. + expect(res.body).to.be.an('object'); + expect(res.body.items).to.be.an('array').that.is.not.empty; }); - it('401s on POST deactivate with JWT auth (same IMS-only contract)', async () => { - const http = getHttpClient(); - const res = await http.admin.post( - `/v2/orgs/${ORG_1_ID}/brands/${BRAND_1_ID}/serenity/deactivate`, - {}, - ); - expect(res.status).to.equal(401); + it('GET /serenity/languages returns 200 with the language catalog', async () => { + const res = await getHttpClient().admin.get(`/v2/orgs/${ORG_1_ID}/serenity/languages`); + expect(res.status).to.equal(200); + expect(res.body).to.be.an('object'); }); + }); - it('400s on non-UUID brandId for activate (route gate, before auth)', async () => { - const http = getHttpClient(); - const res = await http.admin.post( - `/v2/orgs/${ORG_1_ID}/brands/not-a-uuid/serenity/activate`, - { brandDomain: 'example.com', brandNames: ['Example'], markets: [{ market: 'US', languageCode: 'en' }] }, + describe('Serenity API — relaxed auth reaches the handler', () => { + // Before SERENITY_ALLOW_NON_IMS_AUTH the harness's JWT deterministically + // 401'd at requireImsBearer. With the flag, the same call now passes auth + // and proceeds to brand resolution: an unknown brand under an accessible org + // resolves to 404 (NOT 401), proving the relaxed path reaches the handler. + it('brand-level GET markets returns 404 for an unknown brand (not 401)', async () => { + const unknownBrand = '99999999-9999-4999-b999-999999999999'; + const res = await getHttpClient().admin.get( + `/v2/orgs/${ORG_1_ID}/brands/${unknownBrand}/serenity/markets`, ); - expect(res.status).to.equal(400); + expect(res.status).to.equal(404); }); }); } From 00785467dd89660943ce20cc4e3cc2c65d2b9076 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 14:33:50 +0200 Subject: [PATCH 4/8] =?UTF-8?q?refactor(test):=20rename=20startPostgres/st?= =?UTF-8?q?opPostgres=20=E2=86=92=20startContainers/stopContainers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The IT harness now boots the full stack (Postgres + PostgREST, MinIO, and the two Semrush mocks), so the postgres-specific name was misleading. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/it/postgres/harness.js | 6 +++--- test/it/postgres/setup.js | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/it/postgres/harness.js b/test/it/postgres/harness.js index 75932ecbb..94d626167 100644 --- a/test/it/postgres/harness.js +++ b/test/it/postgres/harness.js @@ -24,7 +24,7 @@ import { initAuth, createAllTokens } from '../shared/auth.js'; import { buildEnv } from '../env.js'; import { startServer, stopServer } from '../server.js'; import { createHttpClient } from '../shared/http-client.js'; -import { startPostgres, stopPostgres } from './setup.js'; +import { startContainers, stopContainers } from './setup.js'; /** Shared state populated during beforeAll, consumed by test files. */ export const ctx = {}; @@ -43,7 +43,7 @@ export const mochaHooks = { const { publicKeyB64 } = await initAuth(); const tokens = await createAllTokens(); - await startPostgres(); + await startContainers(); const env = buildEnv(publicKeyB64); const baseUrl = await startServer(env); @@ -56,6 +56,6 @@ export const mochaHooks = { async afterAll() { this.timeout(HARNESS_HOOK_TIMEOUT_MS); await stopServer(); - await stopPostgres(); + await stopContainers(); }, }; diff --git a/test/it/postgres/setup.js b/test/it/postgres/setup.js index f5c509733..da281a4d4 100644 --- a/test/it/postgres/setup.js +++ b/test/it/postgres/setup.js @@ -168,11 +168,13 @@ async function ensureMinIoBucket() { } /** - * Starts PostgreSQL + PostgREST + MinIO via docker compose and waits for readiness. + * Starts the full IT container stack via docker compose — PostgreSQL + PostgREST + * (data-service), MinIO, and the Semrush Project Engine / User Manager mocks — + * and waits for all of them to become ready. * * @returns {Promise} The PostgREST base URL */ -export async function startPostgres() { +export async function startContainers() { // Pin each Semrush mock image to the version of the typed client we actually // depend on (the mock is published from that same package). Drift here would // silently test a different contract than production ships. A bumped client @@ -198,9 +200,9 @@ export async function startPostgres() { } /** - * Tears down docker compose services and removes volumes. + * Tears down all IT containers (data-service, MinIO, Semrush mocks) and removes volumes. */ -export async function stopPostgres() { +export async function stopContainers() { try { execSync( `docker compose -f "${COMPOSE_FILE}" down -v --remove-orphans`, From 780774df620c3bfe20c37290411a68b4a0989414 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 14:44:18 +0200 Subject: [PATCH 5/8] refactor(serenity): document upstream token validation; fail hard on missing mock tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - requireImsBearer: document that the forwarded IMS bearer is validated AGAIN by the real Semrush gateway on every upstream call — this proxy is a fail-fast shape guard, not the auth boundary. Makes explicit why the test-only IMS-type relaxation never weakens production auth. - Remove the hardcoded `:-1.3.0` fallback on the mock image tags. The compose now uses `${...:?}` and fails hard when the tag is unset, and setup.js throws if the installed client version cannot be resolved. A silent default would test a different version than the client we ship. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/controllers/serenity.js | 15 ++++++++++++--- test/it/postgres/docker-compose.yml | 17 ++++++++++------- test/it/postgres/setup.js | 8 +++++++- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index 52b4fd248..19978feb0 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -181,12 +181,21 @@ function mapError(e, log) { * upstream gateway only understands IMS user tokens; we refuse to forward * anything else. * + * SECURITY MODEL — this proxy is NOT the auth boundary; Semrush is. The bearer + * we forward is validated AGAIN by the real Semrush gateway on every upstream + * call (it rejects an invalid/expired/forged token with 401/403, which the + * transport surfaces as a SerenityTransportError). This local check is only a + * fail-fast + shape guard so we do not forward a token Semrush will obviously + * reject; it never substitutes for the upstream's own validation. + * * Test-only escape hatch: when `SERENITY_ALLOW_NON_IMS_AUTH === 'true'` the * IMS-type check is skipped so an authenticated NON-IMS caller (e.g. the * locally-signed JWT the integration-test harness mints) can reach the - * handlers. This is sound ONLY against the Semrush vendor MOCKS, which do not - * validate the forwarded bearer — the token's value never matters, only that - * an authenticated identity is present. Mirrors `SERENITY_ALLOW_WORKSPACE_DELETE` + * handlers. This is sound because (a) production auth is unaffected — Semrush + * still validates the forwarded token end to end — and (b) the integration + * tests run against the Semrush vendor MOCKS, which intentionally do not + * validate the bearer, so the token's value never matters there, only that an + * authenticated identity is present. Mirrors `SERENITY_ALLOW_WORKSPACE_DELETE` * in rest-transport.js: an explicit opt-in flag that NO deployed environment * sets (it is never written to Vault `dx_mysticat//api-service`); it is * for local + automated E2E only. The Authorization-header requirement still diff --git a/test/it/postgres/docker-compose.yml b/test/it/postgres/docker-compose.yml index f4d031bbc..483fa58ef 100644 --- a/test/it/postgres/docker-compose.yml +++ b/test/it/postgres/docker-compose.yml @@ -74,13 +74,16 @@ services: # Image tag == the installed client dependency version. The mock is published # from the SAME package as the typed client (@adobe/spacecat-shared-*-client), # so testing against a different mock version than the client we ship would be - # an unsound contract test. setup.js reads the installed client version and - # exports SERENITY_PE_MOCK_TAG / SERENITY_UM_MOCK_TAG; the `:-` default keeps a - # manual `docker compose up` working. If a client is bumped to a version whose - # mock image is not yet published, the pull fails loudly — by design, it forces - # the mock to be released in lockstep. + # an unsound contract test. setup.js (startContainers) reads the installed + # client version and exports SERENITY_PE_MOCK_TAG / SERENITY_UM_MOCK_TAG. + # The `:?` form FAILS HARD when the tag is unset — there is deliberately NO + # default: a hardcoded fallback would silently run a different version than the + # client we ship (chaos). If a client is bumped to a version whose mock image + # is not yet published, the pull also fails loudly, forcing the mock to be + # released in lockstep. Always launch via `mise run`/the harness, never a bare + # `docker compose up` (which would have no tag set). project-engine-mock: - image: ghcr.io/adobe/spacecat-shared-project-engine-client-mock:${SERENITY_PE_MOCK_TAG:-1.3.0} + image: ghcr.io/adobe/spacecat-shared-project-engine-client-mock:${SERENITY_PE_MOCK_TAG:?must be set to the installed @adobe/spacecat-shared-project-engine-client version (exported by test/it/postgres/setup.js)} container_name: spacecat-it-pe-mock environment: MOCK_SEED: workspace-with-data @@ -94,7 +97,7 @@ services: start_period: 20s user-manager-mock: - image: ghcr.io/adobe/spacecat-shared-user-manager-client-mock:${SERENITY_UM_MOCK_TAG:-1.3.0} + image: ghcr.io/adobe/spacecat-shared-user-manager-client-mock:${SERENITY_UM_MOCK_TAG:?must be set to the installed @adobe/spacecat-shared-user-manager-client version (exported by test/it/postgres/setup.js)} container_name: spacecat-it-um-mock environment: MOCK_SEED: parent-with-child diff --git a/test/it/postgres/setup.js b/test/it/postgres/setup.js index da281a4d4..bba7b55eb 100644 --- a/test/it/postgres/setup.js +++ b/test/it/postgres/setup.js @@ -33,7 +33,13 @@ const REPO_ROOT = path.join(__dirname, '..', '..', '..'); */ function installedVersion(pkg) { const pkgJson = path.join(REPO_ROOT, 'node_modules', pkg, 'package.json'); - return JSON.parse(readFileSync(pkgJson, 'utf8')).version; + // Let a missing package throw (ENOENT) — fail hard rather than fall back to a + // hardcoded tag, which would silently test a different version than we ship. + const { version } = JSON.parse(readFileSync(pkgJson, 'utf8')); + if (!version || typeof version !== 'string') { + throw new Error(`Could not resolve installed version of ${pkg} for the Semrush mock image tag`); + } + return version; } const POSTGREST_PORT = process.env.IT_POSTGREST_PORT || '3300'; const POSTGREST_URL = `http://localhost:${POSTGREST_PORT}`; From a467c9c37c5378c132b6fc998089e47f30bd2674 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 14:49:20 +0200 Subject: [PATCH 6/8] docs(test): note the serenity IT env vars need no Vault / deployed config Co-Authored-By: Claude Opus 4.8 (1M context) --- test/it/env.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/it/env.js b/test/it/env.js index 66f8f3e1e..a0a290297 100644 --- a/test/it/env.js +++ b/test/it/env.js @@ -100,6 +100,8 @@ export function buildEnv(publicKeyB64) { POSTGREST_API_KEY: POSTGREST_WRITER_JWT, // ── Serenity E2E: Semrush vendor mocks ────────────────────────────────── + // NONE of the vars below require Vault / deployed-env config: SEMRUSH_USERS_BASE_URL + // falls back to SEMRUSH_PROJECTS_BASE_URL when unset, and the rest are IT-only. // Point the two serenity transport gateways at the mock containers. The // User Manager origin is split out via SEMRUSH_USERS_BASE_URL (api-service#2656) // so the two mocks need no path-routing reverse proxy. Both serve self-signed From 9749265f8f9bea3cf1e6ba6b9e824d4b63d06f41 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 15:16:03 +0200 Subject: [PATCH 7/8] docs(test): document the Semrush vendor mocks in the IT README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a "Serenity E2E" section explaining how the two Counterfact mock containers are wired here (version-pinned image tags, base-URL split, the test-only auth flag, statefulness/reset, seed alignment) and where they are developed — the upstream spacecat-shared mock docs and source. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/it/README.md | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/test/it/README.md b/test/it/README.md index 675ba67c8..1489a0d1d 100644 --- a/test/it/README.md +++ b/test/it/README.md @@ -38,9 +38,9 @@ test/it/ │ └── postgres/ # PostgreSQL backend ├── harness.js # Mocha root hooks (beforeAll/afterAll) - ├── setup.js # Docker Compose startup + PostgREST polling + ├── setup.js # Container stack startup/poll + Semrush mock reset/version-pinning ├── seed.js # TRUNCATE + re-seed via PostgREST HTTP API - ├── docker-compose.yml # PostgreSQL + PostgREST containers + ├── docker-compose.yml # PostgreSQL + PostgREST + MinIO + Semrush vendor mocks ├── .mocharc.postgres.yml # Mocha config ├── seed-data/ # Seed data in snake_case │ ├── organizations.js @@ -187,6 +187,37 @@ Each `describe` block calls `before(() => resetData())` which truncates all data **PostgreSQL seed** (`postgres/seed.js`): POSTs rows directly to PostgREST (snake_case). Also seeds entities like `async_jobs`. +## Serenity E2E: Semrush vendor mocks + +The `/serenity/*` routes proxy to two external Semrush APIs (Project Engine + User Manager). To exercise them end to end — without a real IMS account or a live vendor — the harness boots **two stateful [Counterfact](https://counterfact.dev) mock containers**, published as **public GHCR images** from the same `spacecat-shared` packages that provide the typed clients: + +| Service (compose) | Image | Serves prefix | Host port | +|---|---|---|---| +| `project-engine-mock` | `ghcr.io/adobe/spacecat-shared-project-engine-client-mock` | `/enterprise/projects/api` | 8443 | +| `user-manager-mock` | `ghcr.io/adobe/spacecat-shared-user-manager-client-mock` | `/enterprise/users/api` | 8444 | + +The images are public, so **no registry credentials are needed** for them (only the data-service image still needs ECR). Each serves self-signed HTTPS (Caddy → Counterfact on `:4010` internally). + +### How it's wired here + +- **Image tag = the installed client version.** `setup.js` (`installedVersion()`) reads the installed `@adobe/spacecat-shared-{project-engine,user-manager}-client` version and exports `SERENITY_PE_MOCK_TAG` / `SERENITY_UM_MOCK_TAG`, which `docker-compose.yml` interpolates. The mock is built from the same package as the client, so this guarantees the test runs against the contract version we ship. There is **no fallback** — the compose `${...:?}` form fails hard if the tag is unset, and a client bumped to a version whose mock image isn't published yet fails the pull (forcing the mock to be released in lockstep). +- **Transport target (`env.js`):** `SEMRUSH_PROJECTS_BASE_URL` → PE mock, `SEMRUSH_USERS_BASE_URL` → UM mock (the User-Manager origin split landed in api-service#2656; it falls back to the projects host when unset, so production needs no new config). `NODE_TLS_REJECT_UNAUTHORIZED=0` trusts the self-signed certs (IT process only). **None of these need Vault / deployed-env config.** +- **Auth (`SERENITY_ALLOW_NON_IMS_AUTH=true`, test-only):** the serenity controller normally forwards only IMS-typed tokens. This flag lets the harness's non-IMS JWT through. It's sound only against the mocks (which ignore the forwarded bearer); in production the real Semrush gateway validates the token end to end, so this never weakens deployed auth. The flag is never set in any deployed environment. +- **Statefulness / isolation:** the mocks are stateful within a run. Auth-exempt control routes — `POST //__reset`, `POST //__seed`, `GET //__dump` — manage that; `setup.js` exposes `resetSemrushMocks()` for tests that mutate mock state (used by the upcoming activate/deactivate + market-create flows). Readiness is polled via `waitForSemrushMocks()`. +- **Seed alignment:** the mock fixtures use fixed workspace/project UUIDs, so the api-service seed (org `semrush_workspace_id` = the mock parent workspace, brand `semrush_workspace_id` = the mock child) is aligned to them. See the upstream `mock/seeds.js`. +- **Test factory:** `shared/tests/serenity.js`, wired in `postgres/serenity.test.js`. + +### Where the mocks are developed (and how to learn more) + +The mocks live in **`adobe/spacecat-shared`**, inside each client package (`packages/spacecat-shared-{project-engine,user-manager}-client/`). Authoritative docs are on that repo's `main`: + +- `docs/mock-usage.md` — control routes (`__reset`/`__seed`/`__dump`/`__quota`), auth behaviour, seed selection (`MOCK_SEED`). +- `docs/mock-statefulness.md` — the in-memory store model and how writes persist within a run. +- `docs/mock-docker.md` — the Docker image, `MOCK_SEED` / `MOCK_SEED_FILE` env, and the intended consumer (GitHub Actions `services:`) shape. +- Package `README.md` — `npm run mock` (local Counterfact on `:4010`), `docker:build` / `docker:run`, and the spec→mock pipeline. +- Source: `mock/` (`run.js`, `seeds.js`, `factories.js`, `stateful.js`, `store.js`, `quota.js`). +- CI: `.github/workflows/{project-engine,user-manager}-client-mock-image.yaml` (publish on release), `*-mock-image-smoke.yaml` (PR smoke), `*-mock-e2e.yaml`. + ## Running Locally ### Prerequisites From 40366566cd629331394dd9d0591914237d158317 Mon Sep 17 00:00:00 2001 From: Rainer Friederich Date: Fri, 26 Jun 2026 15:16:08 +0200 Subject: [PATCH 8/8] fix(serenity): address MysticatBot review on #2709 - resetSemrushMocks: throw on a non-2xx reset instead of swallowing it, so the mutating-lifecycle increment can't produce flaky order-dependent tests. - requireImsBearer flag: log once (warn) when SERENITY_ALLOW_NON_IMS_AUTH is enabled, so an accidental deployed-env enablement is visible in the logs. - IT: strengthen the /serenity/languages assertion to check the { items: [...] } shape, and add a second brand-level e2e (GET prompts) for route breadth. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/controllers/serenity.js | 9 +++++++++ test/it/postgres/setup.js | 10 +++++++++- test/it/shared/tests/serenity.js | 16 +++++++++++++++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index 19978feb0..bbdd12155 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -237,6 +237,11 @@ export function brandPointerReloader(ctx, brandUuid) { }; } +// Logged at most once per process: makes an accidental SERENITY_ALLOW_NON_IMS_AUTH +// enablement in a deployed environment visible in the logs (the flag bypasses the +// IMS-type gate — it must only ever be set for local/automated E2E). +let warnedNonImsAuth = false; + function SerenityController(context, log, env) { if (!isNonEmptyObject(context)) { throw new Error('Context required'); @@ -244,6 +249,10 @@ function SerenityController(context, log, env) { if (!log) { throw new Error('Log required'); } + if (!warnedNonImsAuth && (context?.env || env)?.SERENITY_ALLOW_NON_IMS_AUTH === 'true') { + warnedNonImsAuth = true; + log.warn('[serenity] SERENITY_ALLOW_NON_IMS_AUTH is enabled — the IMS-type auth gate is bypassed. This is test-only and must never be set in a deployed environment.'); + } /** * Verifies the caller has access to the addressed org AND the brand diff --git a/test/it/postgres/setup.js b/test/it/postgres/setup.js index bba7b55eb..57d000ab9 100644 --- a/test/it/postgres/setup.js +++ b/test/it/postgres/setup.js @@ -142,8 +142,16 @@ export async function resetSemrushMocks() { const prev = process.env.NODE_TLS_REJECT_UNAUTHORIZED; process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; try { + // Throw on a failed reset rather than swallow it: a silently-failed reset + // would leave mutated mock state behind and produce flaky, order-dependent + // tests (the mutating-lifecycle increment relies on this). await Promise.all( - MOCK_RESET_PATHS.map((url) => fetch(url, { method: 'POST' }).catch(() => {})), + MOCK_RESET_PATHS.map(async (url) => { + const res = await fetch(url, { method: 'POST' }); + if (!res.ok) { + throw new Error(`Semrush mock reset failed (${res.status}) at ${url}`); + } + }), ); } finally { if (prev === undefined) { diff --git a/test/it/shared/tests/serenity.js b/test/it/shared/tests/serenity.js index 3a92a0fc5..dea7d3bf6 100644 --- a/test/it/shared/tests/serenity.js +++ b/test/it/shared/tests/serenity.js @@ -84,7 +84,10 @@ export default function serenityTests(getHttpClient, resetData) { it('GET /serenity/languages returns 200 with the language catalog', async () => { const res = await getHttpClient().admin.get(`/v2/orgs/${ORG_1_ID}/serenity/languages`); expect(res.status).to.equal(200); + // Same { items: [...] } envelope as models; asserting the shape (not just + // "an object") catches schema drift / an error body slipping through as 200. expect(res.body).to.be.an('object'); + expect(res.body.items).to.be.an('array').that.is.not.empty; }); }); @@ -93,12 +96,23 @@ export default function serenityTests(getHttpClient, resetData) { // 401'd at requireImsBearer. With the flag, the same call now passes auth // and proceeds to brand resolution: an unknown brand under an accessible org // resolves to 404 (NOT 401), proving the relaxed path reaches the handler. + const unknownBrand = '99999999-9999-4999-b999-999999999999'; + it('brand-level GET markets returns 404 for an unknown brand (not 401)', async () => { - const unknownBrand = '99999999-9999-4999-b999-999999999999'; const res = await getHttpClient().admin.get( `/v2/orgs/${ORG_1_ID}/brands/${unknownBrand}/serenity/markets`, ); expect(res.status).to.equal(404); }); + + // A second brand-level route for breadth: a different controller method + // (listPrompts) carrying a query string still routes, passes the relaxed + // auth, and 404s on the unknown brand — not 401, not a 500 from the query. + it('brand-level GET prompts returns 404 for an unknown brand (not 401)', async () => { + const res = await getHttpClient().admin.get( + `/v2/orgs/${ORG_1_ID}/brands/${unknownBrand}/serenity/prompts?geoTargetId=2840&languageCode=en`, + ); + expect(res.status).to.equal(404); + }); }); }