Repository navigation
fix(identity): two #2552 before(*) pin regressions — CronService stall + XsuaaToken getter - #2838
Merged
Merged
Conversation
The #2552 global identity-pin `before('*')` hook (srv/server.js) was registered on EVERY ApplicationService, including the internal CronService. On a scheduled tick the hook calls `pinResolvedUser(req.user, cds.tx(req))`, and `cds.tx(req)` dereferences `ctx.context._set` on the cron event's context — which is not a real EventContext — throwing `ctx.context._set is not a function` on EVERY tick. The throw is caught fail-open (WARN), so jobs still run, but: - it spams the PROD log on every scheduled tick, and - after an instance restart (e.g. a blue-green swap) resets the canary clocks, the noise coincides with the scheduler-stall canary (#2782) firing a false ALERT. The pin is a pure no-op on a cron tick anyway — a scheduled tick's user is always system/anonymous. Skip CronService by its fully-qualified name (com.sap.developers.ims.CronService) in the hook-registration loop. Regression first observed on PROD after the 1.40.1 blue-green swap; the hook + the throwing `cds.tx(req)` form both shipped in 1.40.0 (a3412e0 / 60c6a5b). Test: extend test/hybrid/cron-service-schedule.test.js to assert the identity-pin before(*) handler is NOT registered on CronService. Runs in the post-deploy hybrid CI job (needs a HANA bind profile; not runnable in a fresh worktree without `npm run bind:setup`). Follow-up (separate issue): the same #2552 hook also throws `Cannot set property userId of #<XsuaaToken> which has only a getter` on every authenticated request in PROD — a distinct bug in pinResolvedUser's write to the read-only XsuaaToken getter, not addressed here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ser (#2552) Second #2552 regression, surfaced in PROD logs during the 1.40.1 scheduler diagnosis: on EVERY authenticated request, the identity-pin hook threw `Cannot set property userId of #<XsuaaToken> which has only a getter`. Root cause: pinResolvedUser pinned the resolved sapId by assigning `user.authInfo.token.userId = row.sapId`. In PROD the real @sap/xssec XsuaaToken exposes `userId` as a GETTER-ONLY property, so the bare assignment throws. Caught fail-open (WARN), but it fired on every authed request and the pin silently never cached anything on the XSUAA path. Fix: cache the resolved sapId on `user.attr.sapId` (always a plain writable object) as the source of truth, and only mirror onto `authInfo.token.userId` when that slot is actually writable (checked via getOwnPropertyDescriptor, wrapped defensively). The idempotency short-circuit now reads `user.attr.sapId` first, falling back to the token for older pins. The dev/basic-auth path (writable token) still mirrors userId as before. Test: 4 new cases in test/unit/resolve-db-user-resolve.test.js exercising a getter-only token (reproducing the XsuaaToken), asserting no throw, sapId cached on attr, idempotent re-call, and the writable-token mirror path. All 34 tests in the file pass (`vitest run`). Paired with the CronService guard in this PR — both are #2552 before('*') identity-pin regressions from 1.40.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
jung-thomas
changed the title
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
1 participant
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.
Fix: two #2552 identity-pin regressions (CronService stall + XsuaaToken getter)
Both bugs come from the #2552 global identity-pin
before('*')hook introduced in 1.40.0. Surfaced during the 1.40.1 PROD scheduler-failure diagnosis.Bug 1 — scheduler stall (CronService)
[before *] pin failed { service: 'com.sap.developers.ims.CronService', msg: 'ctx.context._set is not a function' }on every scheduled tick. The hook is registered on every ApplicationService including the internal CronService; a cron tick has no HTTP request, socds.tx(req)dereferencesctx.context._seton a non-EventContextand throws. Fail-open (WARN), but it spams the log and made the #2782 stall canary fire a false ALERT after the 1.40.1 swap reset the canary clocks.Fix: skip
com.sap.developers.ims.CronServicein the hook-registration loop (srv/server.js). The pin is a no-op on a cron tick anyway (user is always system/anonymous).Bug 2 — XsuaaToken getter (every authenticated request)
Cannot set property userId of #<XsuaaToken> which has only a getteron every authed PROD request.pinResolvedUserpinned sapId viauser.authInfo.token.userId = row.sapId, but the real @sap/xssec XsuaaToken exposesuserIdas a getter-only property. Fail-open, but the pin silently never cached on the XSUAA path.Fix: cache sapId on
user.attr.sapId(always writable) as the source of truth; only mirror ontotoken.userIdwhen that slot is writable (checked viagetOwnPropertyDescriptor, wrapped defensively). Idempotency check readsattr.sapIdfirst. Dev/basic-auth (writable token) still mirrors as before. Source lives inpackages/core/resolve-db-user.js; the deploy bundle (srv/lib/_shared/core.bundle.mjs, gitignored) regenerates from it at build.Tests
test/unit/resolve-db-user-resolve.test.js(getter-only token → no throw, sapId cached on attr, idempotent, writable-token mirror). All 34 tests in the file pass locally (vitest run). ✅test/hybrid/cron-service-schedule.test.jsthat the pin hook is not registered on CronService.Interim PROD mitigation (already done)
Rolling-restarted both PROD
tutorials-srvinstances;metrics-rollupconfirmed running again at 21:35Z. Temporary — warnings re-fire until this deploys.🤖 Generated with Claude Code