Skip to content

fix: bound pagination and guard password file reads#61

Open
merlinsantiago982-cmd wants to merge 1 commit into
TestSprite:mainfrom
merlinsantiago982-cmd:fix/pagination-password-file-guards
Open

fix: bound pagination and guard password file reads#61
merlinsantiago982-cmd wants to merge 1 commit into
TestSprite:mainfrom
merlinsantiago982-cmd:fix/pagination-password-file-guards

Conversation

@merlinsantiago982-cmd

@merlinsantiago982-cmd merlinsantiago982-cmd commented Jun 30, 2026

Copy link
Copy Markdown

Summary

  • stop auto-pagination when the server repeats a cursor or exceeds a hard page budget
  • guard --password-file reads with stat-first validation, regular-file checks, and a 64 KiB size cap
  • add tests for cursor cycles, runaway pagination, valid password files, directory rejection, and oversized password files

Why

A malicious or broken server could keep returning non-null cursors indefinitely, causing the CLI to loop and grow memory. --password-file also read arbitrary paths without checking the target type or size before sending the content as the project password.

Testing

  • npm test -- src/lib/pagination.test.ts src/commands/project.test.ts
  • npm run typecheck
  • npm run format:check
  • npm run lint
  • npm run build

Fixes #58

Summary by CodeRabbit

  • New Features

    • Added safer handling for password files when creating or updating projects, including file validation and a size limit.
    • Added a limit for automatic pagination to prevent runaway requests.
  • Bug Fixes

    • Rejects invalid password-file paths, oversized password files, and repeated pagination cursors before making network calls.
    • Prevents infinite or excessive auto-pagination by stopping at a configured maximum page count.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2153754e-f02f-4df4-9d92-bdc19ef58a08

📥 Commits

Reviewing files that changed from the base of the PR and between 15e95de and 75448b3.

📒 Files selected for processing (4)
  • src/commands/project.test.ts
  • src/commands/project.ts
  • src/lib/pagination.test.ts
  • src/lib/pagination.ts

📝 Walkthrough

Walkthrough

Adds two client-side safety mechanisms: paginate() in src/lib/pagination.ts now enforces a MAX_AUTO_PAGES (1000) cap and detects repeated nextToken cursors, throwing structured ApiError instances on violation. The --password-file option in src/commands/project.ts is guarded by readPasswordFileGuarded, which validates the path, enforces MAX_PASSWORD_FILE_BYTES (64 KiB), and returns structured errors. Tests cover all new failure paths.

Changes

Pagination Safety Guards

Layer / File(s) Summary
paginate() safety state and error helper
src/lib/pagination.ts
Imports ApiError, exports MAX_AUTO_PAGES=1000, initializes pagesFetched and seenNextTokens in the loop, throws on cap exceeded or repeated cursor via new paginationSafetyError helper.
Rejection tests
src/lib/pagination.test.ts
Adds tests asserting paginate rejects with UNAVAILABLE/repeated_next_token on cursor cycles and UNAVAILABLE/max_pages_exceeded when MAX_AUTO_PAGES is hit, with call-count assertions.

Password-File Validation

Layer / File(s) Summary
readPasswordFileGuarded helper
src/commands/project.ts
Adds statSync/resolve imports, exports MAX_PASSWORD_FILE_BYTES (64 KiB), implements readPasswordFileGuarded with absolute-path resolution, regular-file check, size cap, and passwordFileError producing typed ApiError envelopes. Wires into runCreate and runUpdate.
Password-file tests
src/commands/project.test.ts
Adds tests for successful read/trim, directory-path VALIDATION_ERROR, and oversized-file PAYLOAD_TOO_LARGE, all asserting no network call on failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop hop, no infinite loop today,
The cursor won't spin us away.
A 64 KiB cap on passwords tight,
And pages stop at one thousand, right.
This bunny guards the CLI with care! 🛡️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 clearly summarizes the two main hardening changes: pagination limits and guarded password-file reads.
Linked Issues check ✅ Passed The PR implements the requested pagination loop protections and password-file validation limits for create and update, matching #58.
Out of Scope Changes check ✅ Passed The changes stay within the security-hardening scope, with only supporting tests and constants added.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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.

Unbounded Pagination and Arbitrary File Read via --password-file

1 participant