Skip to content

fix(sdk): expose skipConditionValidation on uploadCDR and uploadFile#108

Open
sophiej-story wants to merge 1 commit into
mainfrom
fix/skip-condition-validation-upload
Open

fix(sdk): expose skipConditionValidation on uploadCDR and uploadFile#108
sophiej-story wants to merge 1 commit into
mainfrom
fix/skip-condition-validation-upload

Conversation

@sophiej-story

Copy link
Copy Markdown
Collaborator

Why

Closes #50.

uploadCDR() and uploadFile() both call allocate() internally, which validates that the configured condition addresses implement checkWriteCondition / checkReadCondition. allocate() already exposes skipConditionValidation to bypass that check — needed for the CDR contract's msg.sender == conditionAddr short-circuit (CDR.sol write() line 174, read() line 217) when callers use a plain EOA / wallet address as the condition. The high-level wrappers didn't forward the flag, so the EOA-as-condition pattern wasn't reachable through them and would always throw InvalidConditionContractError.

Change

  • packages/sdk/src/uploader.ts — add optional skipConditionValidation?: boolean to uploadCDR() and uploadFile() param types; forward to the internal allocate() call. JSDoc matches the existing comment on allocate().
  • packages/sdk/__tests__/uploader.test.ts — two new tests for uploadCDR: pass-through with EOA addresses asserts simulateContract is not called and the run completes; default path with a non-ContractFunctionRevertedError cause throws InvalidConditionContractError and no tx is sent.
  • packages/sdk/__tests__/upload-file.test.ts — mirror tests for uploadFile, exercising the full encryptFile → storage.upload → allocate → tdh2Encrypt → write pipeline so the skip assertion runs against a fully-completed call.
  • docs/CONDITIONS.md — new "Bypassing Validation for EOA / Wallet-Address Conditions" subsection under "How Conditions Work", explaining the bypass and showing the flag on uploadCDR.

No behavior change when the flag is omitted (default false).

Test plan

  • `pnpm --filter @piplabs/cdr-sdk test` — 187 passed (+4 new), 0 failures
  • `pnpm --filter @piplabs/cdr-sdk lint` — clean
  • PR CI green

@jinn-agent

jinn-agent Bot commented May 22, 2026

Copy link
Copy Markdown

The change is minimal and correct — skipConditionValidation is plumbed through uploadCDR and uploadFile to allocate() with no branching logic added in the wrappers, and the existing allocate() guard already handles the flag. Tests cover both the bypass path and the default rejection path for each wrapper.

One pre-existing issue that becomes more visible with this PR: in uploadFile, the encrypted file is uploaded to the storage provider (Step 2) before allocate() runs condition validation (Step 4). If condition validation throws InvalidConditionContractError, the uploaded ciphertext is orphaned in storage with no associated vault. This is not introduced by this PR, but the new negative test in upload-file.test.ts implicitly confirms the behavior — storageProvider.upload is called before the error surfaces. Worth noting for a follow-up (reordering Steps or adding a pre-flight validation path before the upload).

No high-confidence correctness or security bugs were found in the changed lines.


Review iteration 1 · Commit b0817eb · 2026-05-22T04:17:22Z

@sophiej-story sophiej-story self-assigned this May 22, 2026
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.

uploadCDR and uploadFile should support skipConditionValidation parameter

1 participant