Repository navigation
fix(codeql): triage CodeQL alerts and repair password-recovery reveal flow - #22
Merged
Merged
Conversation
- 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).
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.
Summary
build-mode: noneso source-generator output underobj/(OpenApiXmlCommentSupport.generated.cs, 8 alerts) is no longer analyzed.paths-ignoredoes not apply to compiled languages, so the exclusion happens in the workflow.cs/useless-assignment-to-local):POST /super/recoveries/{id}/revealgenerated a code and hashed it, but never persisted the hash and marked the request as resolved./auth/resetmatches onhash(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.player.component(js/comparison-between-incompatible-types), manual scope dispose →using(cs/missed-using-statement), catch-all →DbUpdateException(cs/catch-of-all-exceptions), explicit LINQWherein 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)
cs/useless-upcast: cast is required — anonymous-type member initializers reject a barenull(CS0828, verified with a local build).js/clear-text-storageon the user profile: stores a non-secret profile payload (the query taints themustChangePasswordboolean).js/clear-text-storageon 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: cleanprettier --check: clean · tsc verified the removed condition was redundant