spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

JWT tool: in-flight signature computation writes stale signature over cleared state (also reddens the full-suite SonarQube scan on master) #829

New Issue
Closed
opened 2026-07-24 19:44:01 +00:00 by spikerj · 3 comments
spikerj commented 2026-07-24 19:44:01 +00:00
Owner
Copy Link
Copy Source

Symptom

The SonarQube Scan job has failed on essentially every spikersoft-angular master run today. It is the only job that runs pnpm nx run-many --target=test --coverage (the FULL suite across all 81 projects) — test-and-lint runs nx affected on PRs, so this failure is invisible until after merge. One of the two failures:

FAIL  feature-dev-tools-encoding  libraries/features/dev-tools-encoding/src/lib/jwt/jwt.component.spec.ts > JwtComponent > should clear all state
AssertionError: expected 'KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJ…' to be '' // Object.is equality
+ KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30

This is a real application bug, not a flaky test. The spec passes in isolation and fails under load only because load changes the timing — but the same race is reachable by a user.

Root cause

JwtComponent.computeSignatureFromSecret() (jwt.component.ts) is kicked off from an effect() and awaits WebCrypto (importSecretKey → signData). Nothing ties the result back to the state it was computed for:

const secretKey = await this.importSecretKey(secret, decoded.header.alg);
const signatureBuffer = await this.signData(headerPayload, secretKey);
this.computedSignature.set(this.arrayBufferToBase64Url(signatureBuffer));  // unconditional

So if the user hits Clear (or changes the token/secret) while a signing run is in flight, clearAll() sets computedSignature to "" and then the stale in-flight run resolves and writes the old signature back. The JWT tool shows a computed signature for a token that is no longer loaded.

Repro (deterministic, no timing dependence)

  1. component.loadDemoToken()
  2. fixture.detectChanges() — runs the effect, which STARTS the async signing
  3. component.clearAll() — before the WebCrypto promise resolves
  4. await flush()

Expected computedSignature() === ""; actual, the demo token's signature. Verified A/B locally: this fails with the exact CI assertion error on current master and passes with the fix.

Fix

Guard the async write with a monotonic run id: computeSignatureFromSecret captures const run = ++this.signatureRun and drops its writes if run !== this.signatureRun on completion; clearAll() and the effect's empty branch bump the counter to invalidate in-flight work.

## Symptom The `SonarQube Scan` job has failed on essentially every `spikersoft-angular` master run today. It is the only job that runs `pnpm nx run-many --target=test --coverage` (the FULL suite across all 81 projects) — `test-and-lint` runs `nx affected` on PRs, so this failure is invisible until after merge. One of the two failures: ``` FAIL feature-dev-tools-encoding libraries/features/dev-tools-encoding/src/lib/jwt/jwt.component.spec.ts > JwtComponent > should clear all state AssertionError: expected 'KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJ…' to be '' // Object.is equality + KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30 ``` **This is a real application bug, not a flaky test.** The spec passes in isolation and fails under load only because load changes the timing — but the same race is reachable by a user. ## Root cause `JwtComponent.computeSignatureFromSecret()` (jwt.component.ts) is kicked off from an `effect()` and awaits WebCrypto (`importSecretKey` → `signData`). Nothing ties the result back to the state it was computed for: ```ts const secretKey = await this.importSecretKey(secret, decoded.header.alg); const signatureBuffer = await this.signData(headerPayload, secretKey); this.computedSignature.set(this.arrayBufferToBase64Url(signatureBuffer)); // unconditional ``` So if the user hits **Clear** (or changes the token/secret) while a signing run is in flight, `clearAll()` sets `computedSignature` to `""` and then the stale in-flight run resolves and writes the old signature back. The JWT tool shows a computed signature for a token that is no longer loaded. ## Repro (deterministic, no timing dependence) 1. `component.loadDemoToken()` 2. `fixture.detectChanges()` — runs the effect, which STARTS the async signing 3. `component.clearAll()` — before the WebCrypto promise resolves 4. `await flush()` Expected `computedSignature() === ""`; actual, the demo token's signature. Verified A/B locally: this fails with the exact CI assertion error on current master and passes with the fix. ## Fix Guard the async write with a monotonic run id: `computeSignatureFromSecret` captures `const run = ++this.signatureRun` and drops its writes if `run !== this.signatureRun` on completion; `clearAll()` and the effect's empty branch bump the counter to invalidate in-flight work.
spikerj commented 2026-07-24 19:45:56 +00:00
Author
Owner
Copy Link
Copy Source

Fix up in spikersoft-angular PR #570 (fix/sonar-suite-red-jwt-race) — awaiting CI/review.

computeSignatureFromSecret() now captures a monotonic signatureRun id and drops its computedSignature / isComputingSignature writes if a newer run (or a clear) superseded it mid-await; clearAll() and the effect's empty branch bump the counter to invalidate work in flight.

The regression test is deterministic rather than timing-dependent: loadDemoToken() → detectChanges() (this is what actually STARTS the async signing) → clearAll() before the WebCrypto promise resolves → flush. A/B verified: on unfixed master it fails with CI's exact assertion (expected 'KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJ…' to be ''); with the fix, 31/31 pass.

One correction to my own first attempt, recorded so the next person does not repeat it: my initial version of the guard called clearAll() immediately after loadDemoToken() without detectChanges(). That test passed against the unfixed code — Angular effects are scheduled, not synchronous, so no computation had started and there was nothing in flight to race. A regression guard for an async race has to prove it fails without the fix, or it is measuring nothing.

Full local nx run-many --target=test is green across all 81 projects with this change.

— Opus 5 Agent

Fix up in spikersoft-angular **PR #570** (`fix/sonar-suite-red-jwt-race`) — awaiting CI/review. `computeSignatureFromSecret()` now captures a monotonic `signatureRun` id and drops its `computedSignature` / `isComputingSignature` writes if a newer run (or a clear) superseded it mid-await; `clearAll()` and the effect's empty branch bump the counter to invalidate work in flight. The regression test is deterministic rather than timing-dependent: `loadDemoToken()` → `detectChanges()` (this is what actually STARTS the async signing) → `clearAll()` before the WebCrypto promise resolves → flush. **A/B verified:** on unfixed master it fails with CI's exact assertion (`expected 'KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJ…' to be ''`); with the fix, 31/31 pass. One correction to my own first attempt, recorded so the next person does not repeat it: my initial version of the guard called `clearAll()` immediately after `loadDemoToken()` **without** `detectChanges()`. That test passed against the *unfixed* code — Angular effects are scheduled, not synchronous, so no computation had started and there was nothing in flight to race. A regression guard for an async race has to prove it fails without the fix, or it is measuring nothing. Full local `nx run-many --target=test` is green across all 81 projects with this change. — Opus 5 Agent
spikerj commented 2026-07-24 19:58:38 +00:00
Author
Owner
Copy Link
Copy Source

CI evidence that the fix works: on PR #570 the SonarQube Scan job passed — the first green Sonar scan on this repo today, after failing on every master run. All other jobs on #570 are green too (test-and-lint, build, e2e-anonymous, e2e-smoke).

PR #570 is still open pending review/merge.

— Opus 5 Agent

CI evidence that the fix works: on PR #570 the **`SonarQube Scan` job passed** — the first green Sonar scan on this repo today, after failing on every master run. All other jobs on #570 are green too (`test-and-lint`, `build`, `e2e-anonymous`, `e2e-smoke`). PR #570 is still open pending review/merge. — Opus 5 Agent
spikerj commented 2026-07-24 20:45:53 +00:00
Author
Owner
Copy Link
Copy Source

Fixed and verified on master. spikersoft-angular PR #570 merged (b3626e69).

The evidence this ticket actually needed is the master-side scan, not the PR-branch one: master run 18137 on 721d171b has SonarQube Scan = success. That is the first green full-suite scan on master today, after it failed on every master run beforehand. test-and-lint, build and e2e-smoke are green on the same run.

Verified in the merged code: computeSignatureFromSecret() captures a monotonic signatureRun and drops its computedSignature / isComputingSignature writes when superseded; clearAll() and the effect's empty branch bump the counter to invalidate work in flight. The deterministic regression guard is in place (A/B-verified: fails on the unfixed code with this ticket's exact assertion string).

Worth restating for the record, since it is the part that outlives the fix: the failing spec was a real user-reachable race, not a flaky test. Clearing the JWT tool while a WebCrypto signing run was in flight repopulated the cleared field. CI load only changed the timing that exposed it.

Closing.

— Opus 5 Agent

Fixed and verified on master. spikersoft-angular PR #570 merged (`b3626e69`). The evidence this ticket actually needed is the **master-side** scan, not the PR-branch one: master run 18137 on `721d171b` has **`SonarQube Scan` = success**. That is the first green full-suite scan on master today, after it failed on every master run beforehand. `test-and-lint`, `build` and `e2e-smoke` are green on the same run. Verified in the merged code: `computeSignatureFromSecret()` captures a monotonic `signatureRun` and drops its `computedSignature` / `isComputingSignature` writes when superseded; `clearAll()` and the effect's empty branch bump the counter to invalidate work in flight. The deterministic regression guard is in place (A/B-verified: fails on the unfixed code with this ticket's exact assertion string). Worth restating for the record, since it is the part that outlives the fix: the failing spec was a **real user-reachable race**, not a flaky test. Clearing the JWT tool while a WebCrypto signing run was in flight repopulated the cleared field. CI load only changed the timing that exposed it. Closing. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#829