Skip to content
Open
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
12 changes: 10 additions & 2 deletions .dev/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,15 @@ The `SearchClient` abstraction already exists as the right boundary; the migrati

### GraphQL server migration (away from Apollo)

_Priority: medium. Blocked by, or done in parallel with, the core module extraction._
_Priority: high. prerequisite for Keycloak auth implementation._

Apollo Server 3 is end-of-life. Upgrading to Apollo Server 4 is not the right move; the goal is to move _away_ from Apollo, not deeper into it. Apollo is opinionated about its hosting environment (it assumes Express-style middleware, has its own context and plugin APIs) in ways that conflict with the longer-term direction of decoupling Arranger from any specific framework.
Apollo Server 3 is end-of-life. Upgrading to Apollo Server 4 is not the right move; Apollo is opinionated about its hosting environment (it assumes Express-style middleware, has its own context and plugin APIs) in ways that conflict with the longer-term direction of making Arranger framework-agnostic.

**Sequencing:** This item is the second step of a three-step chain driven by a pentest audit finding:

1. **Disable introspection in Apollo v3 (done):** `introspection: process.env.NODE_ENV !== 'production'` added to both `ApolloServer` constructors in `graphqlRoutes.ts`. Apollo v4+ does this automatically when `NODE_ENV=production`; this change aligns v3 behaviour with that convention so consumers are not surprised after the migration. Note: field name suggestions in GraphQL error responses are a separate gap not yet addressed; Apollo v3 requires a custom `formatError` to suppress them, Apollo v4 handles it automatically. This remaining gap closes when the migration lands.
2. **Migrate to graphql-yoga (this item):** Clean foundation before auth is built on Apollo-specific APIs.
3. **Keycloak bearer token auth (follows):** Implemented as Express middleware - library-agnostic, portable across the migration. Direct Keycloak JWT validation while Usher planning continues. See [Auth and field/record-level access control](#auth-and-fieldrecord-level-access-control).

The leading replacement candidate is **graphql-yoga** (maintained by The Guild, who also maintain `@graphql-tools`, already used in this repo). It runs on any JS runtime, integrates with Express without requiring it, supports the same schema-first approach the codebase uses, and is actively maintained. This is a research-confirmed candidate, not a final decision.

Expand Down Expand Up @@ -763,6 +769,8 @@ _The CI pod already has dind; testcontainers would work today. Evaluate as part

Catches phantom dependencies at install time, faster CI installs, removes `dangerouslyDisablePackageManagerCheck`.

**Corepack Docker gotcha (learned from Lyric):** When adopting pnpm with corepack in a multi-stage Dockerfile, use `corepack prepare pnpm@x.y.z --activate` - not `corepack use pnpm@x.y.z`. `corepack use` only updates package.json; the binary is fetched lazily at runtime. If the pod has no egress to registry.npmjs.org (common in locked-down clusters), startup fails with a corepack download error. `corepack prepare --activate` downloads and caches the binary during image build, so the pod needs no network access to start.

1. Install pnpm on Jenkins nodes (coordinate with infra)
2. Create `pnpm-workspace.yaml` (replacing npm workspaces declaration)
3. Run `pnpm import` to generate `pnpm-lock.yaml`
Expand Down
20 changes: 20 additions & 0 deletions .dev/sessions.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ Search engine startup errors now tell operators exactly what went wrong and what

---

## 2026-06-25

Disabled GraphQL introspection in Apollo v3 to close a pentest audit finding; elevated Apollo migration to high priority and documented the introspection fix → migration → Keycloak auth chain in the roadmap.

- `modules/graphql-router/src/graphqlRoutes.ts`: added `disableGraphQLIntrospection` param to `createEndpoint` and `ArrangerRoutesArgs`/`arrangerRoutes`; named to avoid ambiguity with the server's own REST introspection routes
- `modules/graphql-router/src/disableGraphQLIntrospection.test.ts`: unit tests for both flag states (introspection allowed and rejected); co-located at `src/` level following convention (`disablePlayground.test.ts` is in `__tests__/` which is the inconsistency flagged in tech-debt)
- `integration-tests/server/test/spinupActive.js`: test 8 verifies introspection returns schema data in non-production mode
- `modules/types/src/configs/constants.ts`: added `DISABLE_GRAPHQL_INTROSPECTION` to `configArrangerFeatureFlagProperties`; `featureFlagDefaults` and `ConfigsObject` pick it up automatically
- `modules/graphql-router/src/graphqlRoutes.ts`: `createEndpoint` call reads `configs[DISABLE_GRAPHQL_INTROSPECTION] ?? false`;
- `apps/search-server/src/configs/fromEnv/localEnvs.ts`: reads `DISABLE_GRAPHQL_INTROSPECTION` env var with `NODE_ENV === 'production'` as fallback; consistent with other feature flags in the same file
- `apps/search-server/configTemplates/configs.json.schema`: added `disableGraphQLIntrospection` to the feature flags section
- `apps/search-server/src/configs/fromEnv/aggregator.ts`: added `disableGraphQLIntrospection` to `externalConfigs` destructuring and `catalogs.fromEnv`; enables passing the flag programmatically to `ArrangerServer` for integration-level testing
- `integration-tests/server/test/index.test.ts`: added `GraphQL introspection disabled` suite; starts server with `disableGraphQLIntrospection: true` via `ExternalConfigs` and asserts introspection queries return 400 with an introspection-related error message
- `.dev/roadmap.md`: GraphQL server migration raised from medium to high priority; sequencing note added covering the three-step chain and the remaining field-suggestions gap (closes automatically on migration)
- `.dev/tech-debt.md`: updated "GraphQL introspection" entry (now only field suggestions remains); added new entry for network aggregation `__type` dependency on remote nodes
- `modules/graphql-router/README.md`: added `disableGraphQLIntrospection` to feature flags table; added introspection requirement caveat in the Network search section
- `docs/usage/04-introspection.md`: added "GraphQL introspection" section distinguishing the REST API from GraphQL's introspection system; documents the flag, env var, and network aggregation caveat

---

## 2026-06-24

Added `getAllData` to the `graphql-router` utils barrel export (was missing, causing `ERR_PACKAGE_PATH_NOT_EXPORTED` for a downstream consumer); extended the `integration-tests/import` tech-debt entry with two explicit gaps.
Expand Down
19 changes: 14 additions & 5 deletions .dev/tech-debt.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,14 @@ When Arranger Server (`apps/search-server`) is updated to use `catalogue`, the M
**Fix:** Strip stack traces from client-facing error responses in the GraphQL error formatter. Optionally surface them when `enableDebug` is true (server-side only) or when `enableAdmin` is active; but that dependency on the Admin model is TBD. Safe default: never send stacks to clients.
**Standalone:** mostly yes; the stack stripping is a one-file fix. The question of whether debug mode re-enables stack visibility is the only part that touches the Admin design.

### GraphQL introspection and field suggestions in production
### GraphQL field suggestions leak schema structure in error responses

**File:** `modules/graphql-router/src/graphqlRoutes.ts` (Apollo Server config)
**Severity:** medium (OWASP A02: Security Misconfiguration)
**Severity:** low (OWASP A02: Security Misconfiguration)
**Kind:** security bug
**Issue:** Arranger exposes field name suggestions in error messages and may allow full introspection queries in production. Both leak schema structure to clients. Introspection should be disabled in production (it is already gated by `disablePlayground`, but the introspection query and field suggestion behaviour may be independent of that flag). Possibly a side-effect of using outdated Apollo Server 3 without explicit introspection controls.
**Fix:** Explicitly disable introspection and field suggestions in production. Apollo Server 3 supports `introspection: false` and `stopSuggestions` via the `graphql` validation layer. Verify these are configured correctly and not accidentally left open. This may partially resolve itself when Apollo is replaced; but the config intent should be documented regardless.
**Standalone:** mostly yes for the immediate fix; deeper fix is part of the Apollo migration
**Issue:** Apollo Server 3 includes field name suggestions in error messages (e.g. "Did you mean 'fieldName'?"). This leaks schema structure to clients through error responses even when `disableGraphQLIntrospection: true` is set; suggestions are a separate code path from the introspection system. Note: GraphQL introspection itself is now gated by `disableGraphQLIntrospection`, which defaults to `true` in production via `NODE_ENV`.
**Fix:** Override `formatError` in `createEndpoint` to strip suggestion hints from Apollo error messages before they reach clients, or add a custom validation rule that suppresses them. This will resolve itself on the yoga migration (yoga does not expose suggestions by default), but the interim fix is small and standalone.
**Standalone:** yes; isolated change to the Apollo Server config in `createEndpoint`

### `hasValidConfig` GraphQL resolver should be deprecated

Expand Down Expand Up @@ -304,6 +304,15 @@ When Arranger Server (`apps/search-server`) is updated to use `catalogue`, the M
**Issue:** `resolveSetsInSqon` has two paths - SQON contains no `set_id:` values (no-op, returns SQON unchanged) and SQON contains `set_id:` values (expands to stored IDs via an ES search). Neither path has a unit test.
**Standalone:** yes; but note the file also carries the `hackyTemporaryEsSetResolution` tech-debt entry; evaluate for removal during Sets full-feature implementation rather than investing deeply in tests for code that may be replaced

### Network aggregation schema discovery depends on GraphQL introspection being open on remote nodes

**File:** `modules/graphql-router/src/network/setup/query.ts` (`fetchNodeAggregations`)
**Severity:** medium
**Kind:** design coupling / security constraint
**Issue:** At startup, Arranger queries each remote node using `__type(name: $documentTypeName)` to discover its aggregation field types. `__type` is part of the GraphQL introspection system. If a remote node has `disableGraphQLIntrospection: true`, its schema discovery fails and the node is excluded from federation with a `NETWORK_ERROR` or `INVALID_DATA` result. This creates a conflict: hardening any node in a network aggregation deployment breaks the federation setup for nodes pointing at it.
**Fix:** Replace the `__type`-based discovery with a call to the REST `/introspection/fields` (or `/introspection/:catalogueId`) endpoint already provided by `apps/search-server`. That endpoint returns equivalent field information without requiring GraphQL introspection to be open. Natural task within the GraphQL server migration; coordinate with yoga switchover so both changes land together.
**Standalone:** no; the REST introspection endpoint must be stable and reachable from the aggregating node's network context; coordinate with the yoga migration

### `hackyTemporaryEsSetResolution.js`: stale ES 6.2 workaround + convention violation

**File:** `modules/graphql-router/src/mapping/hackyTemporaryEsSetResolution.js`
Expand Down
1 change: 1 addition & 0 deletions apps/search-server/configTemplates/configs.json.schema
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

"disableDownloads": false,
"disableFilters": false,
"disableGraphQLIntrospection": false,
"disablePlayground": false,
"disableSets": false,

Expand Down
4 changes: 4 additions & 0 deletions apps/search-server/src/configs/fromEnv/aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const configsAggregator = (
catalogConfigsPath,
disableDownloads,
disableFilters,
disableGraphQLIntrospection,
disablePlayground,
disableSets,
enableAdmin,
Expand All @@ -25,13 +26,16 @@ const configsAggregator = (
setsType,
} = externalConfigs;

// lodash merge skips missing externalConfigs values, falling back to the localEnvs default.
// and first empty {} prevents mutating the configsFromLocalEnv module singleton.
const aggregatedEnvConfigs = merge({}, configsFromLocalEnv, {
allowedCorsOrigins,
catalogConfigsPath,
catalogs: {
fromEnv: {
disableDownloads,
disableFilters,
disableGraphQLIntrospection,
disablePlayground,
disableSets,
enableAdmin,
Expand Down
5 changes: 5 additions & 0 deletions apps/search-server/src/configs/fromEnv/localEnvs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import {
} from '@overture-stack/arranger-types/configs/constants';
import { stringToBool, stringToNumber } from '@overture-stack/arranger-types/tools';

// TODO: make a more robust isProd helper (e.g. casing + alternatives like 'prod')
const isProd = process.env.NODE_ENV === 'production';

const configsFromEnv = {
allowedCorsOrigins: process.env.ALLOWED_CORS_ORIGINS?.split(',')
.map((origin) => origin.trim())
Expand All @@ -21,6 +24,8 @@ const configsFromEnv = {
// feature flags
[configFeatureFlagProperties.DISABLE_DOWNLOADS]: stringToBool(process.env.DISABLE_DOWNLOADS),
[configFeatureFlagProperties.DISABLE_FILTERS]: stringToBool(process.env.DISABLE_FILTERS),
[configFeatureFlagProperties.DISABLE_GRAPHQL_INTROSPECTION]:
stringToBool(process.env.DISABLE_GRAPHQL_INTROSPECTION) ?? isProd,
[configFeatureFlagProperties.DISABLE_GRAPHQL_PLAYGROUND]: stringToBool(
process.env.DISABLE_GRAPHQL_PLAYGROUND,
),
Expand Down
1 change: 1 addition & 0 deletions apps/search-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const arrangerServer = async ({ esClient, ...externalConfigs }: ExternalConfigs)
try {
const { allowedCorsOrigins, catalogs, enableDebug, enableLogs, health, serverPort } =
await loadAllConfigs(externalConfigs);

const catalogEntries = Object.entries(catalogs);
const catalogMode = catalogEntries.length > 1 ? 'multiple' : 'single';

Expand Down
15 changes: 15 additions & 0 deletions docs/usage/04-introspection.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,18 @@ Returns the SQON JSON Schema and operator metadata shared across all catalogues
Use this to validate or describe SQON structure independently of any specific catalogue's field set. For field-specific operator applicability, use `/introspection/:catalogueId` instead.

See [SQONs in detail](./03-sqon-in-detail.md) for full documentation of the SQON filter language.

---

## GraphQL introspection

The REST endpoints above are Arranger's own introspection API and are always available regardless of GraphQL settings. GraphQL's built-in introspection system (`__schema`, `__type` queries) is a separate capability that Arranger gates with the `disableGraphQLIntrospection` flag.

By default, GraphQL introspection is disabled when `NODE_ENV=production` and enabled otherwise. You can also control it explicitly:

- In a catalogue config file (`base.json`): `"disableGraphQLIntrospection": true`
- Via environment variable: `DISABLE_GRAPHQL_INTROSPECTION=true`

Disabling GraphQL introspection is recommended in production to avoid exposing schema structure to clients through `__schema`/`__type` queries (OWASP A02).

**Caveat - network aggregation:** When network search federation is active, Arranger queries each remote node's GraphQL endpoint using `__type` to discover its aggregation field types at startup. If a remote node has GraphQL introspection disabled, that node's schema discovery fails and it is silently excluded from federation. Until this dependency is replaced with a REST-based discovery call, do not set `disableGraphQLIntrospection: true` on any node that serves as a remote target in a network aggregation deployment.
51 changes: 50 additions & 1 deletion integration-tests/server/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { after, before, suite } from 'node:test';
import assert from 'node:assert/strict';
import { after, before, suite, test } from 'node:test';
import path from 'path';

import { stringToNumber } from '@overture-stack/arranger-types/tools';
import axios from 'axios';
import dotenv from 'dotenv';

import ArrangerServer from '../../../apps/search-server/src/server.js';
Expand Down Expand Up @@ -315,6 +317,53 @@ suite('integration-tests/server', { concurrency: false }, () => {
});
});

suite('GraphQL introspection disabled', () => {
let serverApp;

before(async () => {
console.error('\n------------------------------------');
console.log('Setting up Arranger - Introspection Disabled\n');

try {
serverApp = await ArrangerServer({
disableGraphQLIntrospection: true,
enableAdmin,
esClient,
serverPort,
setsIndex,
setsType,
});
} catch (err) {
console.error('\n\n------------------------------------');
console.error('FATAL: Arranger Server is not available - aborting tests\n');
console.error(` ${err instanceof Error ? err.stack : err}\n`);
console.error('------------------------------------\n');
process.exit(1);
}
});

test('rejects GraphQL introspection queries with a 400 response', async () => {
const response = await axios.post(
`${serverUrl}/graphql`,
{ query: '{ __schema { queryType { name } } }' },
{ validateStatus: () => true },
);

assert.equal(response.status, 400);
assert.ok(Array.isArray(response.data?.errors) && response.data.errors.length > 0);
assert.match(response.data.errors[0].message, /introspection/i);
});

after(async () => {
try {
serverApp.close();
console.log('\nStopped Arranger Server - Introspection Disabled\n');
} catch (err) {
// console.log('err after', err);
}
});
});

after(async () => {
try {
await cleanup('after all');
Expand Down
18 changes: 18 additions & 0 deletions integration-tests/server/test/spinupActive.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,5 +116,23 @@ export default ({ api, catalogs, mode = 'single' }) => {
}
});

test('8.should allow GraphQL introspection in non-production mode', async () => {
for (const { gqlPath } of catalogs) {
const { data, statusText } = await api
.post({
body: {
query: `{ __schema { queryType { name } } }`,
},
endpoint: gqlPath,
})
.catch((err) => {
console.log('spinupActive/graphql-introspection error', err.message);
});

assert.equal(statusText, 'OK');
assert.ok(data?.data?.__schema?.queryType?.name, 'introspection should return schema data when not in production');
}
});

// TODO: add /download checks
};
5 changes: 4 additions & 1 deletion modules/graphql-router/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ const router = await arrangerRouter(options);
|---|---|---|---|
| `disableDownloads` | `boolean` | `false` | Disable the TSV/file download endpoint. |
| `disableFilters` | `boolean` | `false` | Disable SQON filter support on queries. |
| `disableSets` | `boolean` | `false` | Disable saved Sets. |
| `disableGraphQLIntrospection` | `boolean` | `false` (`true` when `NODE_ENV=production`) | Disable GraphQL's built-in `__schema`/`__type` introspection system. Recommended in production. **Caveat:** remote nodes used in a [network aggregation](#network-search) deployment must keep this disabled; see that section for details. |
| `disablePlayground` | `boolean` | `false` | Disable the GraphQL Playground UI. |
| `disableSets` | `boolean` | `false` | Disable saved Sets. |

### Table

Expand Down Expand Up @@ -161,6 +162,8 @@ When using `apps/search-server`, this config lives in `network.json` inside the

All nodes must serve overlapping index field names. Fields with the same name and GraphQL type are merged across nodes; fields unique to one node are excluded from federation.

**Introspection requirement:** At startup, each remote node's aggregation field types are discovered via a `__type` GraphQL introspection query. Remote nodes that have `disableGraphQLIntrospection: true` will fail schema discovery and be silently excluded from federation. Do not enable `disableGraphQLIntrospection` on any node that serves as a remote target in a network aggregation deployment. A fix that replaces this with a REST `/introspection/fields` call is tracked in tech-debt and planned for the yoga migration.

---

## Server-side filters
Expand Down
Loading