Route the backend on BTP, close the app's security gaps, consume the cds-plugin - #23
Merged
Merged
Conversation
ROUTING (P0) -- the deployed app could not have worked. mta.yaml has always declared an abap2UI5-srv destination pointing at the CAP module, and xs-app.json never routed anything to it: its last route is a catch-all to the static HTML5 repository, so every POST /rest/root/z2ui5 roundtrip went to the file store instead of the service. Nothing noticed, because jest runs against a local cds server that never sees the approuter and deploy-check deliberately skips mbt build. Routes added for the roundtrip, the OData service and /health; src/test/approuter-routes.test.js pins the contract, the order (backend routes must beat the catch-all), the auth types, and that every destination a route names exists in mta.yaml. csrfProtection is false on the roundtrip route deliberately: the webapp posts with no CSRF token and never fetches one, so the approuter's default would 403 every roundtrip. CSRF is defended one layer down instead -- see below. SECURITY (P1): - The services now require the declared `User` role instead of merely `authenticated-user`. xs-security.json had declared the $XSAPPNAME.User scope and a role template since the beginning and nothing referenced them, so the authorization model existed on paper while every authenticated user in the subaccount passed, role collection assigned or not. The mocked development users carry the same role, so local and test flows are unchanged. - Security headers now reach the DATA endpoints. Only the bootstrap page applied the framework's t_security_header; the roundtrip and the OData entities -- the responses that actually carry application state -- answered with no nosniff, no frame guard, and cacheable by any intermediary. - The request body is capped explicitly (Z2UI5_MAX_BODY, default 2mb). The action takes an @OPEN object, so CDS validates nothing, and the only limit was express's 100kb default applying by accident rather than by decision. - Draft retention had disagreed with the framework about its own TTL -- this job deleted at 24h while the exit's draft_exp_time_in_hours said 4h, with nothing connecting them. The framework value is now the source of truth and the env var overrides both. It also ran on every instance, so N instances meant N concurrent hourly DELETEs over the same rows; it now runs on one (Z2UI5_DRAFT_RETENTION_INSTANCE, default CF instance 0). - Indexes for the two non-primary-key access paths, as HANA design-time artifacts in db/src: createdAt, because the hourly retention DELETE was a full scan whose cost grew with the very table it exists to contain, and owner, because the service projection filters on it. Verified to reach gen/db/src through `cds build --production`. - mta.yaml declares instances: 1. Sticky app state lives in the serving process with no session affinity configured, so scaling out produces intermittent, hard-to-reproduce state loss. The comment says what to fix before raising it. - dependabot gains the npm ecosystem in the app repo, which it never had -- no runtime dependency of the deployed app was ever proposed for update. CAP packages are grouped (they version together); openui5-dist is ignored, because that pin is a compatibility decision tracked upstream. - npm audit runs on the shipped lock in the app repo's CI, advisory for now. README's security section documented the role and CSRF gaps honestly; it now documents what replaced them, and the sticky/multi-instance limitation that remains. builder 30 tests, app 44 tests (was 23), lint clean, cds build --production ok. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2gZuy95uKcT2zngQd1sAL
The package now ships its CAP wiring (cds-plugin.js -> srv/cap/activate.js) and its CDS model, so this app stopped hand-rolling all of it: - srv/server.js drops ~120 lines -- identity, the draft store, retention, the GET/HEAD bootstrap routes and the /resources mount all come from the plugin. What is left is genuinely app-specific: srv/app discovery, the request body cap, security headers on the data endpoints, and /health. - srv/draft-retention.js is deleted; the package ships it. - db/schema.cds and srv/z2ui5-service.cds import the packaged model and service rather than redeclaring them, which is the same import an external project writes -- so this app's test suite is what proves the packaged definitions work, instead of them being exercised only in theory. - srv/z2ui5-service.js no longer registers the roundtrip handler; the plugin does, on whichever service declares a z2ui5 action. That last point was not cosmetic. CAP auto-loads a dependency's cds-plugin, so with the app ALSO doing the wiring, retention started twice -- two hourly DELETE loops over the same rows in one process. Now once, asserted. Also resolves a contradiction two published documents had about the one folder users are told to write in: the README said "put your own apps into srv/app/" while AGENTS.md said srv/app/ is overwritten on every publish. Both now say the same thing, with a table that answers it by what the reader is doing, and the README opens by stating that this repository is the demo rather than the delivery mechanism. builder 30 tests, app 44 tests, lint clean, cds build --production ok. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2gZuy95uKcT2zngQd1sAL
Picks up the CP/NP/IN comparison fixes, the escaping helpers and the CAP entry points. builder 30 tests, app 44 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2gZuy95uKcT2zngQd1sAL
Picks up the removal of upstream's S-RTTI package (12 unused classes whose factory returned null for every type kind) and the repair of z2ui5_cl_util_json_fltr. Core drops from 318 to 306 files. builder 30 tests, app 44 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2gZuy95uKcT2zngQd1sAL
Picks up the CP/NP/IN lowering finally reaching the samples tree, the brace-balanced calling-convention parser, and the frontend patcher that fails instead of silently shipping unpatched. builder 30 tests, app 44 tests, lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2gZuy95uKcT2zngQd1sAL
Picks up the exported UNCAUGHT_EXCEPTION_PREFIX contract the smoke gate now reads instead of a copied literal. builder 30 tests, app 44 tests, lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T2gZuy95uKcT2zngQd1sAL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The deployed app could not have worked
mta.yamlhas always declared anabap2UI5-srvdestination pointing at the CAP module, andxs-app.jsonnever routed anything to it. Its last route is a catch-all to the static HTML5 repository, so on a real BTP deployment everyPOST /rest/root/z2ui5roundtrip went to the file store instead of the service — that is the entire application.It was invisible because jest runs against a local
cdsserver that never sees the approuter, anddeploy-checkdeliberately skipsmbt build. Routes added for the roundtrip, the OData service and/health;src/test/approuter-routes.test.jspins the contract, the route order (backend routes must beat the catch-all), the auth types, and that every destination a route names exists inmta.yaml.csrfProtection: falseon the roundtrip route is deliberate: the webapp posts with no CSRF token and never fetches one, so the approuter's default would 403 every roundtrip. CSRF is defended one layer down — see below.Security
Userrole is now required instead of bareauthenticated-user.xs-security.jsonhad declared the$XSAPPNAME.Userscope and a role template since the beginning and nothing referenced them, so the authorization model existed on paper while every authenticated user in the subaccount passed, role collection assigned or not.nosniff, no frame guard, and cacheable by any intermediary.Z2UI5_MAX_BODY, default 2mb). The action takes an@openobject so CDS validates nothing, and the only limit was express's 100kb default applying by accident.draft_exp_time_in_hourssaid 4h, with nothing connecting them. The framework value is now the source of truth. It also ran on every instance, so N instances meant N concurrent hourlyDELETEs over the same rows.createdAt, because the hourly retention delete was a full scan whose cost grew with the very table it exists to contain, andowner, because the service projection filters on it. Verified to reachgen/db/srcthroughcds build --production.instances: 1inmta.yaml: sticky app state lives in the serving process with no session affinity configured, so scaling out produces intermittent, hard-to-reproduce state loss. The comment says what to fix before raising it.The app stops duplicating the framework's wiring
The package now ships its CAP wiring (
cds-plugin.js→srv/cap/activate.js) and its CDS model, sosrv/server.jsdrops ~120 lines,srv/draft-retention.jsis deleted, anddb/schema.cds/srv/z2ui5-service.cdsimport the packaged definitions rather than redeclaring them — the same import an external project writes, so this app's suite is what proves they work.That last point was not cosmetic: CAP auto-loads a dependency's
cds-plugin, so with the app also doing the wiring, retention started twice — two hourlyDELETEloops over the same rows in one process. Now once, asserted.A contradiction between two published documents
The README said "put your own apps into
srv/app/" whileAGENTS.mdsaidsrv/app/is overwritten on every publish. Both now say the same thing, with a table answering it by what the reader is doing, and the README opens by stating that the generated repository is the demo, not the delivery mechanism.builder 30 tests, app 44 tests (was 23), lint clean,
cds build --productionok.Generated by Claude Code
Generated by Claude Code