Uh oh!
There was an error while loading. Please reload this page.
test(agent-bff): pin the OpenAPI mount point under /agent and cover mode 2 - #1827
Conversation
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
2eab2d9 to
1f5e04dCompare…ode 2 The BFF has no deny-by-default gate: auth only applies under /agent, so a route mounted elsewhere would serve the document unauthenticated. Pin the invariant statically (served path, importers of src/openapi, public surface) and add the missing api key coverage on the route. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1f5e04d to
413ec62Compare| function relativeImportsOf(file: string): string[] { | ||
| const source = readFileSync(path.join(SRC_DIR, file), 'utf8'); | ||
| return [...source.matchAll(/from '(\.[^']*)'/g)].map(match => |
There was a problem hiding this comment.
The scanner only matches static single-quoted from '...' imports, so await import('../openapi/openapi-document') slips through unflagged and nothing in the lint config forbids it. What about widening the regex to /(?:from|import\s*\(|require\s*\()\s*'(\.[^']*)'/g?
| describe('when the package public surface is read', () => { | ||
| it('should expose nothing OpenAPI-related, since a consumer could mount it off /agent', () => { | ||
| expect(Object.keys(publicApi).filter(name => /openapi/i.test(name))).toEqual([]); |
There was a problem hiding this comment.
This pins the main entry only: with no exports map in package.json and files: dist/**/*.js, a consumer can still deep-import @forestadmin/agent-bff/dist/openapi/openapi-document and serve the document ungated. Adding "exports": { ".": "./dist/index.js" } in a follow-up would close it, or we narrow the test name to the main entry.
Tonours
left a comment
There was a problem hiding this comment.
Verified the mutation-testing claim locally: adding an openapi import outside the allowlist fails the invariant and names the file, and moving OPENAPI_PATH to the root breaks 18 tests. Two non-blocking suggestions inline on the scanner regex and the deep-import surface.
Uh oh!
There was an error while loading. Please reload this page.
Why
The BFF has no deny-by-default gate: auth applies only to paths under
/agent, and/healthis a hardcoded public exact-match ahead of the chain.GET /agent/openapi.jsonis therefore gated by construction, not by a mechanism — which means the next route added elsewhere could serve the document unauthenticated, silently. An unauthenticated OpenAPI document hands over the full API surface (collections, fields, filter operators, actions) — OWASP API8:2023.PRD-886 already covers 401-without-credential, 200-with-a-session-token and 404-at-the-root. This PR adds what unit tests did not yet pin: the invariant itself, and the Mode 2 path.
What
test/openapi/openapi-mount-invariant.test.ts(static, no server boot):OPENAPI_PATHmust start with/agent/.src/outsidesrc/openapi/may importsrc/openapi/*, except the known mount points:cli-core.ts->openapi-routes,unfolded-document, andcli-dispatch.ts->openapi-document,unfolded-document,unfolding. Asserted as an exact map, so any new importer fails — the first CI run on this branch demonstrated it by catching the importers PRD-684 added onmain.src/index.tsexports nothing matching/openapi/i, so a package consumer cannot mount the document itself.test/openapi/openapi-routes.test.ts: the missing Mode 2 coverage — a valid API key gets 200 and the very same unfolded document a session caller gets, and 404openapi_disabledwith a valid API key whenBFF_OPENAPI_ENABLED=false.Test-only. No production code touched.
Verification
yarn workspace @forestadmin/agent-bff test— 1084 tests green; lint clean.OPENAPI_PATHto the root, adding anopenapi-documentimport in another module, and exportingcreateOpenApiRoutesfromindex.tseach fail the suite. The Mode 2 cases fail if key resolution fails, so the 200 does not pass for the wrong reason./openapi.json404. Logged on the ticket.Out of scope
A generic deny-by-default gate for the whole BFF, and pinning its public HTTP surface. Recorded as still-open in a ticket comment.
Fixes PRD-686
🤖 Generated with Claude Code