Skip to content

fix: correct base64 fallback decoding and use crypto random for padding#68

Open
drago1520 wants to merge 1 commit intodraphy:masterfrom
drago1520:fix/base64-fallback-and-padding-rng
Open

fix: correct base64 fallback decoding and use crypto random for padding#68
drago1520 wants to merge 1 commit intodraphy:masterfrom
drago1520:fix/base64-fallback-and-padding-rng

Conversation

@drago1520
Copy link
Copy Markdown

What changed

Three small hardening changes in @pushforge/builder:

  1. Fixed base64 decode fallback in Node.
  2. Added regression tests for exact decoded byte length.
  3. Replaced Math.random() padding-size selection with crypto RNG.

1. Base64 decode fix

File:

Old Node fallback did this:

return Buffer.from(base64, 'base64').buffer;

Problem:

  • Buffer is often view into larger backing ArrayBuffer.
  • .buffer returns whole backing memory, not exact decoded bytes.

So caller could get wrong ArrayBuffer.byteLength.

New code slices exact region:

const buffer = Buffer.from(base64, 'base64');
return buffer.buffer.slice(
  buffer.byteOffset,
  buffer.byteOffset + buffer.byteLength,
);

Also tightened runtime checks:

  • typeof globalThis.atob === 'function'
  • typeof globalThis.btoa === 'function'

This avoids entering browser path when property exists but is not callable.

2. Regression tests

File:

Added tests for:

  • normal decode path returns exact 1-byte result
  • forced Buffer fallback path also returns exact 1-byte result

These tests protect against future regressions in base64 decode behavior.

3. Crypto RNG for padding size

File:

Old code used:

Math.random()

New code uses:

crypto.getRandomValues(new Uint8Array(1))[0]

Why:

  • padding randomness should come from cryptographic RNG, same as other sensitive random values in this package
  • small cleanup, more consistent with security-sensitive code

Verification

Ran locally:

pnpm build
pnpm -C packages/builder exec vitest run test/unit.test.ts

Results:

  • build passed
  • unit tests passed: 48/48

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