Skip to content

fix(storage): validate types when reading session storage (Closes #144) - #175

Merged
harsharajkumar-273 merged 1 commit into
harsharajkumar-273:mainfrom
SakethSumanBathini:fix/144-validate-session-storage-types
Aug 4, 2026
Merged

fix(storage): validate types when reading session storage (Closes #144)#175
harsharajkumar-273 merged 1 commit into
harsharajkumar-273:mainfrom
SakethSumanBathini:fix/144-validate-session-storage-types

Conversation

@SakethSumanBathini

Copy link
Copy Markdown
Contributor

Closes #144

Problem

getSelectedRepo tested truthiness, not type:

if (!parsed?.owner || !parsed?.name || !parsed?.fullName) return null;

That rejects '' and undefined but accepts true, 42, {} or an array. The value is then returned typed as Repository, and the first consumer to call a string method on it throws — EditorPage.tsx:1057 reads it straight into the editor's boot path.

sessionStorage is writable by any script on the origin and survives reloads, so a single malformed value wedges the app on every subsequent load until storage is cleared by hand.

stored value          old behaviour              new behaviour
{owner: true, ...}    accepted, then throws      rejected -> null
{owner: 42, ...}      accepted, then throws      rejected -> null
{owner: {}, ...}      accepted, then throws      rejected -> null
valid repo            accepted                   accepted

getTeamSession was weaker still

The issue names both functions, and the second is worth calling out: it checked only parsed?.repo?.fullName, then copied parsed.repo.owner and parsed.repo.name out completely unexamined. A team session whose repo carried a boolean owner passed validation entirely.

Change

A shared parseRepository helper holds all three repository fields to typeof === 'string' && length > 0, and both entry points use it — so the nested repo inside a team session is validated to the same standard as a standalone one.

defaultBranch falls back to 'main' only when it is a non-empty string, rather than on any falsy value. The optional team-session fields are type-checked too: hostName and hostLogin must be non-empty strings and createdAt a finite number, otherwise they come back undefined rather than passing through whatever was stored.

Behaviour for well-formed values is unchanged.

Verification

  • Typechecks clean under --strict.
  • The four malformed-value cases above exercised against both implementations.

Note: npm run build --prefix frontend currently fails on this branch, but for an unrelated pre-existing reason — TS2367 at EditorPage.tsx:3197, which is what my #138 branch fixes. No error is reported in workspaceStorage.ts. Once #138 lands, the build is clean.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@SakethSumanBathini, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: af4f2951-c224-4560-9215-680810527157

📥 Commits

Reviewing files that changed from the base of the PR and between 44cd432 and bc5443e.

📒 Files selected for processing (1)
  • frontend/src/utils/workspaceStorage.ts

Comment @coderabbitai help to get the list of available commands.

@harsharajkumar-273 harsharajkumar-273 added ELUSOC Required Tracking VETERAN Advanced (50 pts) labels Aug 4, 2026
@harsharajkumar-273
harsharajkumar-273 merged commit d964eb9 into harsharajkumar-273:main Aug 4, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ELUSOC Required Tracking VETERAN Advanced (50 pts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Frontend] Session Storage Deserialization Crash on Corrupted Storage Strings

2 participants