Skip to content

fix(security): bound Parquet RLE expansion - #235

Merged
VforVitorio merged 3 commits into
mainfrom
fix/security-parquet-rle-expansion
Nov 2, 2026
Merged

VforVitorio merged 3 commits into
mainfrom
fix/security-parquet-rle-expansion

Conversation

@VforVitorio

@VforVitorio VforVitorio commented Nov 2, 2026 •

Copy link
Copy Markdown
Owner

The client-side Parquet importer preflights page metadata, but Hyparquet 1.26.2 expands an RLE run beyond the declared page value count. A valid 391-byte Parquet file declaring one row can therefore make the browser allocate and write millions of values.

This adds a version-checked postinstall patch that rejects a zero-length or oversized RLE run before the dependency writes to its output array. It applies with both npm and Bun and fails installation if Hyparquet changes and the patch no longer matches.

Related to F1-StratLab issue #227 D2. The issue remains open because its other phases are incomplete.

Verification

  • Reproduced the prior failure with a direct decoder test before applying the guard.
  • Added a regression using the mutated 391-byte Parquet fixture through parseUploadedFile.
  • Added a UI test that verifies a rejected file leaves the previous dataset selected.
  • Webapp suite: 201 tests passed; typecheck, lint, and production build passed.
  • Headless Chromium loaded a CSV, rejected the RLE Parquet file, and retained the CSV dataset with no page errors.
  • The install hook was exercised through npm and Bun.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of malformed Parquet uploads. Files with invalid RLE data are now rejected, and rejecting one leaves the previously uploaded dataset unchanged.
    • Added safeguards against invalid run counts during Parquet decoding to prevent malformed data from being processed.

Reject hybrid RLE runs that exceed the decoded value count before the dependency can extend its output array.
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: db649d29-7a12-43e1-90e7-8679b50a276b
Commits

Reviewing files that changed from the base of the PR and between d65c03c and ddf02c7.

Files selected for processing: 3
  • .github/workflows/webapp-ci.yml
  • webapp/Dockerfile
  • webapp/patch-hyparquet-rle.mjs
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 294fca51-ed34-443a-bcd0-280015649d9a
Commits

Reviewing files that changed from the base of the PR and between a34e23f and d65c03c.

Files selected for processing: 3
  • webapp/package.json
  • webapp/patch-hyparquet-rle.mjs
  • webapp/src/features/race/lib/uploadParse.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The webapp runs a version-checked Hyparquet RLE decoder patch after installation. The patch rejects invalid run counts. Tests verify decoder and parser rejection of malformed Parquet data, and confirm that a failed upload leaves the previous dataset unchanged.

Changes: RLE validation

Layer / File(s) Summary
Install-time decoder patch
webapp/patch-hyparquet-rle.mjs, webapp/package.json
The patch script requires Hyparquet version 1.26.2 and replaces the expected decoder snippet with checks for zero or oversized run counts. The postinstall script runs this patch.
Malformed Parquet upload validation
webapp/src/features/race/lib/uploadParse.test.tsx
Tests verify that an oversized RLE run throws without changing the output array, that parsing the malformed fixture rejects, and that the prior dataset remains in the store after the upload error.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Moderate · previous review d65c0

The Docker build invokes a patch script that is not yet present in the image, risking a failed image build. Copy the script before installing dependencies before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "fix(security): bound Parquet RLE expansion" matches the main change. The PR adds a version-checked postinstall patch to Hyparquet's RLE decoder. The patch rejects zero-length or oversized R…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix(security): bound Parquet RLE expansion" matches the main change. The PR adds a version-checked postinstall patch to Hyparquet's RLE decoder. The patch rejects zero-length or oversized RLE runs before they expand the output. The title names the security fix and the affected decoder path. It is short and single-sentence, and it uses a conventional commit prefix.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread webapp/package.json Resolved
Fail installation if both patched and vulnerable decoder blocks are present.
@VforVitorio
VforVitorio merged commit 69a5207 into main Nov 2, 2026
13 checks passed
@VforVitorio
VforVitorio deleted the fix/security-parquet-rle-expansion branch November 2, 2026 18:41
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