Skip to content

fix(codeql): triage CodeQL alerts and repair password-recovery reveal flow - #22

Merged
santidev21 merged 1 commit into
mainfrom
chore/codeql-triage
Oct 1, 2026
Merged

santidev21 merged 1 commit into
mainfrom
chore/codeql-triage

Conversation

@santidev21

Copy link
Copy Markdown
Owner

Summary

  • CodeQL workflow: switch C# analysis to build-mode: none so source-generator output under obj/ (OpenApiXmlCommentSupport.generated.cs, 8 alerts) is no longer analyzed. paths-ignore does not apply to compiled languages, so the exclusion happens in the workflow.
  • Bug fix (alert cs/useless-assignment-to-local): POST /super/recoveries/{id}/reveal generated a code and hashed it, but never persisted the hash and marked the request as resolved. /auth/reset matches on hash(code) && !resolved, so password recovery could never succeed. Reveal now persists the hash of the code actually shown (RecoveryRequest.ReplaceCode) and only resolves on successful reset.
  • Alert fixes: redundant null check in player.component (js/comparison-between-incompatible-types), manual scope dispose → using (cs/missed-using-statement), catch-all → DbUpdateException (cs/catch-of-all-exceptions), explicit LINQ Where in settings loop and slug filter (cs/linq/missed-where), unused test result now asserted (cs/useless-assignment-to-local).

Dismissals (documented in the alert threads)

  • 10 × cs/useless-upcast: cast is required — anonymous-type member initializers reject a bare null (CS0828, verified with a local build).
  • 3 × js/clear-text-storage on the user profile: stores a non-secret profile payload (the query taints the mustChangePassword boolean).
  • 2 × js/clear-text-storage on tokens: accepted risk, documented (localStorage session tokens + strict CSP; cookie migration is an architecture change).

Testing

  • dotnet build: 0 errors · dotnet test: 13/13 passed · dotnet format whitespace --verify-no-changes: clean
  • ESLint: 0 errors (9 pre-existing warnings) · prettier --check: clean · tsc verified the removed condition was redundant

- codeql.yml: switch C# analysis to build-mode none so source-generator
  output under obj/ (OpenApiXmlCommentSupport) is excluded from analysis;
  paths-ignore does not apply to compiled languages.
- recoveries/{id}/reveal: persist the hash of the code actually shown and
  stop resolving the request on reveal, so /auth/reset can match it
  (the previously stored hash belonged to a code that was never
  disclosed, which made password recovery unusable).
- player.component: drop a null check that is always true after the
  guard above (js/comparison-between-incompatible-types).
- AdminAuthHandler: use a using-scope instead of manual try/finally
  dispose (cs/missed-using-statement).
- MarkIdempotentAsync: catch DbUpdateException instead of all
  exceptions (cs/catch-of-all-exceptions).
- Settings loop and slug filter: use explicit LINQ Where
  (cs/linq/missed-where).
- MatchHistoryTests: assert the closed round instead of leaving the
  result unused (cs/useless-assignment-to-local).
@santidev21
santidev21 merged commit 6378fbb into main Oct 1, 2026
8 checks passed
@santidev21
santidev21 deleted the chore/codeql-triage branch October 1, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant