Potential fix for code scanning alert no. 23: Clear text storage of sensitive information#71
Merged
DarkModder33 merged 1 commit intomainfrom Feb 17, 2026
Merged
Conversation
…ensitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSanitizes leaderboard data before writing it to localStorage by introducing a helper that strips sensitive OAuth identifiers, and updates the leaderboard persistence logic to use this sanitized representation while keeping full entries in memory and for server interactions. Sequence diagram for submitting leaderboard entry with sanitized localStorage cachingsequenceDiagram
actor Player
participant GamePage
participant ServerAPI
participant LocalStorage
Player->>GamePage: submitScore(entry)
GamePage->>ServerAPI: POST /leaderboard with full entry
ServerAPI-->>GamePage: payload.entries (full LeaderboardEntry[])
GamePage->>GamePage: setLeaderboardEntries(payload.entries)
GamePage->>GamePage: topEntries = payload.entries.slice(0, 50)
GamePage->>GamePage: publicTopEntries = topEntries.map(stripSensitiveLeaderboardFields)
GamePage->>LocalStorage: setItem(LEADERBOARD_STORAGE_KEY, JSON.stringify(publicTopEntries))
Player-->>Player: Full entries kept in memory only
Class diagram for sanitized leaderboard persistenceclassDiagram
class LeaderboardEntry {
+string id
+string name
+number score
+string oauthProvider
+string oauthUserId
+string walletAddress
}
class GamePage {
+persistLocal(entry LeaderboardEntry) void
+submit(entry LeaderboardEntry) Promise~void~
+setLeaderboardEntries(entries LeaderboardEntry[]) void
}
class stripSensitiveLeaderboardFields {
+stripSensitiveLeaderboardFields(entry LeaderboardEntry) LeaderboardEntry
}
GamePage --> LeaderboardEntry : uses
GamePage ..> stripSensitiveLeaderboardFields : calls
%% Implementation detail of stripSensitiveLeaderboardFields
class stripSensitiveLeaderboardFieldsImpl {
+oauthProvider undefined
+oauthUserId undefined
}
stripSensitiveLeaderboardFields --> LeaderboardEntry : returns sanitized copy
stripSensitiveLeaderboardFieldsImpl --|> LeaderboardEntry
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
stripSensitiveLeaderboardFieldshelper currently asserts the result asLeaderboardEntrywhile settingoauthProvider/oauthUserIdtoundefined; consider introducing aPublicLeaderboardEntry = Omit<LeaderboardEntry, 'oauthProvider' | 'oauthUserId'>type and using that for localStorage to avoid misleading types and required fields beingundefined. - If there are or will be other persistence paths for leaderboard data, it may be safer to centralize this sanitization in a shared utility (e.g., a
sanitizeLeaderboardForStoragehelper) so that any new storage usage automatically omits sensitive fields.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `stripSensitiveLeaderboardFields` helper currently asserts the result as `LeaderboardEntry` while setting `oauthProvider`/`oauthUserId` to `undefined`; consider introducing a `PublicLeaderboardEntry = Omit<LeaderboardEntry, 'oauthProvider' | 'oauthUserId'>` type and using that for localStorage to avoid misleading types and required fields being `undefined`.
- If there are or will be other persistence paths for leaderboard data, it may be safer to centralize this sanitization in a shared utility (e.g., a `sanitizeLeaderboardForStorage` helper) so that any new storage usage automatically omits sensitive fields.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Potential fix for https://github.com/DarkModder33/main/security/code-scanning/23
In general, the safest approach is to avoid persisting sensitive identifiers in clear text at all. Here, localStorage is used purely as a caching/fallback layer for leaderboard data; there is no strict need to cache
oauthProvideroroauthUserIdlocally. Instead, we can strip these properties (and any other sensitive identity fields we want to treat cautiously) before writing the data to localStorage, while still keeping them in memory and in server submissions.Concretely, in
app/game/GamePageClient.tsxwe will:submissionandentryobjects unchanged at creation time; they still includeoauthProvider,oauthUserId, andwalletAddressso game logic and server API calls remain intact.localStorage(both inpersistLocaland in theresponse.okbranch ofsubmit), map the entries to a “public” shape that omitsoauthProviderandoauthUserId(and any additional sensitive fields we decide to exclude). For example, createpublicEntries = merged.map(stripSensitiveLeaderboardFields)and store that instead.stripSensitiveLeaderboardFieldsinside this component file that takes aLeaderboardEntryand returns a copy with sensitive fields dropped. This function will be used only right before localStorage writes.setLeaderboardEntriesworking with full entries as before, using the original arrays (toporpayload.entries) so that in‑memory behavior is unchanged. Only the data written to localStorage is sanitized.No additional external dependencies are required. The only code additions are the helper function and minor changes in the
persistLocalandsubmitlogic wherelocalStorage.setItemis called.Suggested fixes powered by Copilot Autofix. Review carefully before merging.
Summary by Sourcery
Sanitize leaderboard data before caching it locally to avoid storing sensitive OAuth identifiers in clear text.
Bug Fixes:
Enhancements: