Skip to content

deps(protocol): vendor TypeID generation, drop typeid-js#6103

Open
qw-in wants to merge 1 commit into
mainfrom
quinn/vendor-typeid
Open

deps(protocol): vendor TypeID generation, drop typeid-js#6103
qw-in wants to merge 1 commit into
mainfrom
quinn/vendor-typeid

Conversation

@qw-in

@qw-in qw-in commented Jun 26, 2026

Copy link
Copy Markdown
Member

Replace the external typeid-js dependency with a small, dependency-free inline TypeID generator in @arcjet/protocol. This removes typeid-js and its transitive uuid dependency, resolving the uuid <11.1.1 advisory in the dependency tree (root npm audit: 8 → 6 findings, uuid gone).

We only ever mint new local request IDs with a fixed prefix, so the vendored module covers generation only: a UUIDv7 (RFC 9562) encoded as a 26-character Crockford base32 suffix, matching the TypeID spec (https://github.com/jetify-com/typeid). It mirrors the implementation already vendored and shipping in the Arcjet Python SDK, so both SDKs produce identical IDs.

Tests are ported from the Python SDK's test_typeid.py (format, UUIDv7 version/variant, first-char bound, uniqueness, time-sortability, zero/max timestamp, low/max entropy, alphabet, and fuzz/round-trip invariants). Also drops the now-unused typeid-js alias from the deno example.

Validated: 13 ported tests pass, protocol lint passes, typeid-js + uuid removed from the root and deno lockfiles.


@arcjet-rei would appreciate a review since you added it in arcjet-py!

Replace the external typeid-js dependency with a small, dependency-free
inline TypeID generator in @arcjet/protocol. This removes typeid-js and
its transitive uuid dependency, resolving the uuid <11.1.1 advisory in
the dependency tree (root npm audit: 8 → 6 findings, uuid gone).

We only ever mint new local request IDs with a fixed prefix, so the
vendored module covers generation only: a UUIDv7 (RFC 9562) encoded as a
26-character Crockford base32 suffix, matching the TypeID spec
(https://github.com/jetify-com/typeid). It mirrors the implementation
already vendored and shipping in the Arcjet Python SDK, so both SDKs
produce identical IDs.

Tests are ported from the Python SDK's test_typeid.py (format, UUIDv7
version/variant, first-char bound, uniqueness, time-sortability, zero/max
timestamp, low/max entropy, alphabet, and fuzz/round-trip invariants).
Also drops the now-unused typeid-js alias from the deno example.

Validated: 13 ported tests pass, protocol lint passes, typeid-js + uuid
removed from the root and deno lockfiles.

Co-Authored-By: Claude <noreply@anthropic.com>
@qw-in qw-in requested a review from arcjet-rei June 26, 2026 22:39
@qw-in qw-in self-assigned this Jun 26, 2026
@qw-in qw-in requested a review from a team as a code owner June 26, 2026 22:39
@qw-in qw-in marked this pull request as draft June 26, 2026 22:40

@arcjet-review arcjet-review 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.

Arcjet Review — 🟡 Medium Risk

Decision: Approved

Rationale: This PR removes the external typeid-js dependency and replaces it with a small vendored UUIDv7/TypeID generator used for local request IDs. The dependency-change escalation trigger fires, and the PR changes ID-generation logic, so the risk is Medium rather than Low. I am approving because the implementation is well-scoped, uses Web Crypto CSPRNG randomness, preserves the existing lreq TypeID format at the call site, and adds focused tests for formatting, uniqueness, UUIDv7 version/variant bits, timestamp ordering, timestamp round-tripping, and boundary entropy values. No hardcoded secrets, auth changes, injection risks, or vulnerable new dependencies were identified.

Summary of Changes

Vendors a dependency-free TypeID generator in protocol/typeid.ts, updates ArcjetDecision to use it, removes typeid-js from protocol/package.json and the Deno example import map, and adds tests covering UUIDv7/TypeID invariants.

Escalation Triggers

  • Dependency Changes: protocol/package.json removes the typeid-js dependency, and the Deno example import map also removes typeid-js.

Review Focus Areas

Notes

Security checklist applied: no secrets, credential exposure, injection surface, auth/authorization changes, or insecure randomness were found. The implementation reduces supply-chain surface by removing typeid-js and its transitive uuid dependency.

Path filtering: 2 files excluded by ignore paths. 5 of 7 files included in review.

Review: 320a6531 | Model: openai/gpt-5.5 | Powered by Arcjet Review

Comment thread protocol/typeid.ts
nowMs: number = Date.now(),
random: Uint8Array = randomBytes(),
): Uint8Array {
const timestampMs = BigInt(Math.trunc(nowMs));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please consider validating nowMs is a finite integer in the UUIDv7 timestamp range 0 <= nowMs < 2 ** 48 before converting and shifting it. Because this helper is exported and accepts injectable values, negative or too-large timestamps can wrap when written via setBigUint64, producing IDs with an unexpected embedded timestamp.

Comment thread protocol/typeid.ts
random: Uint8Array = randomBytes(),
): Uint8Array {
const timestampMs = BigInt(Math.trunc(nowMs));
const randA = BigInt((random[0] << 4) | (random[1] >> 4)); // 12 bits

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please consider validating random.length === 10 or keeping this helper internal-only. With a short injected Uint8Array, the first bit operations can silently treat missing bytes as zero and the later BigInt(random[i]) calls can throw, which makes failures less obvious for callers/tests.

@qw-in

qw-in commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Looks like I missed some instances of typeid-js. Will pick those up as well!

Actually, just in examples or arcjet-guard picking it up from the published version

@qw-in qw-in marked this pull request as ready for review June 26, 2026 22:42

@arcjet-review arcjet-review 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.

Arcjet Review — 🟡 Medium Risk

Decision: Approved

Rationale: This PR removes the external typeid-js dependency and vendors a focused TypeID/UUIDv7 generator for local request IDs. The dependency-change escalation trigger fires, and the PR changes ID-generation logic, so the risk remains Medium. I am approving because the implementation is small and well-scoped, uses Web Crypto CSPRNG randomness by default, preserves the lreq TypeID string format at the call site, and adds focused tests for TypeID formatting, UUIDv7 version/variant bits, uniqueness, timestamp ordering, timestamp round-tripping, and entropy boundary cases. No hardcoded secrets, auth/authorization changes, injection surfaces, insecure randomness, or vulnerable new dependencies were identified. The previous review concerns about validating injectable timestamp and random-byte inputs remain unresolved, so I have called them out again as non-blocking inline comments.

Summary of Changes

Replaces the protocol package's typeid-js dependency with a vendored TypeID generator backed by UUIDv7 bytes and Crockford base32 encoding, updates the ArcjetDecision local request ID generation call site, removes typeid-js from package/example dependency declarations, and adds tests covering the new generator.

Escalation Triggers

  • Dependency Changes: protocol/package.json and examples/deno-sensitive-info/deno.json remove the typeid-js dependency and replace it with vendored implementation code.

Review Focus Areas

Notes

Security checklist applied: no secrets, credential exposure, injection risks, auth/authorization changes, or insecure randomness were found. The dependency change reduces supply-chain surface by removing typeid-js and its transitive dependency. Previous review comments on input validation remain unresolved but are not considered blocking for the current production call path, which uses default Date.now() and CSPRNG-generated 10-byte randomness.

Path filtering: 2 files excluded by ignore paths. 5 of 7 files included in review.

Review: 32e2b773 | Model: openai/gpt-5.5 | Powered by Arcjet Review

Comment thread protocol/typeid.ts
nowMs: number = Date.now(),
random: Uint8Array = randomBytes(),
): Uint8Array {
const timestampMs = BigInt(Math.trunc(nowMs));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please consider validating nowMs is a finite integer in the UUIDv7 timestamp range 0 <= nowMs < 2 ** 48 before converting and shifting it. Because this helper is exported and accepts injectable values, negative or too-large timestamps can wrap when written via setBigUint64, producing IDs with an unexpected embedded timestamp.

Comment thread protocol/typeid.ts
random: Uint8Array = randomBytes(),
): Uint8Array {
const timestampMs = BigInt(Math.trunc(nowMs));
const randA = BigInt((random[0] << 4) | (random[1] >> 4)); // 12 bits

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please consider validating random.length === 10 or keeping this helper internal-only. With a short injected Uint8Array, the first bit operations can silently treat missing bytes as zero and the later BigInt(random[i]) calls can throw, which makes failures less obvious for callers/tests.

@arcjet-rei arcjet-rei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a good idea for the same reason that I decided to do something similar in Python – there's a bunch of additional stuff under the TypeID umbrella that we don't use or care about. Assuming the implementation is the same (and, as much as I can tell from eyeballing it, it is), this simplifies maintenance of the SDK and gets rid of some ongoing toil, so I'm all in favor of it. Good idea!

@davidmytton davidmytton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great to drop an external dep!

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.

3 participants