fix(core): use a cryptographically secure source for randomUUID

[adilburaksen]

randomUUID() prefers crypto.randomUUID() and otherwise builds the UUID from
Math.random():

export function randomUUID(): string {
  if (globalThis.crypto?.randomUUID) {
    return globalThis.crypto.randomUUID();
  }
  ...
  const randomValue = (Math.random() * 16) | 0;   // not a CSPRNG

crypto.randomUUID() is a secure-context-only API. On a plain-HTTP origin
globalThis.crypto is still present but crypto.randomUUID is undefined, so the optional
chain short-circuits and every byte of the UUID comes from Math.random(), which is not a
cryptographically secure generator. The degradation is silent — the output has the same
shape and length either way. The same happens on runtimes with no globalThis.crypto
at all (Node 16 and earlier).

This matters because some callers use the result to make security decisions:

• core/src/auth/auth_handler.ts:141 — the OAuth2 state parameter, which
  core/src/auth/oauth2/oauth2_credential_exchanger.ts:167-171 later compares against the
  value returned by the provider. core/src/auth/auth_credential.ts:44 describes this as
  the client being able to "verify the state", so its unpredictability is the property being
  relied on.
• core/src/sessions/in_memory_session_service.ts:63 and
  core/src/sessions/database_session_service.ts:112 — session identifiers.

AuthHandler is reachable from the browser bundle, which is the environment where the
secure-context restriction applies: core/package.json:21 points browser at
dist/web/index_web.js, core/src/index_web.ts:2 re-exports ./common.js, and
core/src/common.ts:90 exports AuthHandler.

Change

Insert crypto.getRandomValues() as the fallback ahead of Math.random(), and throw if
neither is available.

getRandomValues() is deliberately the right fallback here: unlike randomUUID(), it is
not restricted to secure contexts, so it is available in exactly the environments where
randomUUID() is missing. In practice this preserves every environment that works today —
all browsers and Node 18+ expose it — while removing the non-cryptographic path. The UUID is
assembled per RFC 4122 §4.4 (version 4, variant 10xx).

The throw is a last resort for a runtime that offers no CSPRNG at all. Failing closed
seemed better than silently returning a predictable value, but I am happy to change it if
you would rather keep a fallback for such environments — one option is a separate,
explicitly-named non-cryptographic helper for the IDs that do not gate anything (A2A message
and artifact IDs, for example), leaving this function strictly secure.

Tests

Adds coverage for randomUUID() in core/test/utils/env_aware_utils_test.ts, which had
none:

• returns a valid v4 UUID when crypto.randomUUID() exists;
• still returns a valid v4 UUID when randomUUID is absent but getRandomValues is present
  (the plain-HTTP browser case);
• with a stubbed getRandomValues returning fixed bytes, the output is exactly the expected
  UUID — proving the bytes come from getRandomValues and not from Math.random();
• throws when neither API is available.

Context

Prior to #433 the Math.random() construction ran unconditionally, so this is not a
regression from that change — #433 narrowed when the weak path is taken. This PR removes it.

[AmaadMartin]

The security argument holds up and the implementation is correct. I checked the RFC 4122 bit-twiddling ((b & 0x0f) | 0x40 on octet 6, (b & 0x3f) | 0x80 on octet 8) and the byte grouping, and the fixed-byte test's expected abababab-abab-4bab-abab-abababababab is exactly what those bits produce from 0xab in every octet. The fallback is not dead code either: core/build.js:11 targets chrome58/firefox57/safari11, none of which have crypto.randomUUID, all of which have getRandomValues. I also grepped the repo — every UUID caller already routes through this one helper (auth/auth_handler.ts, both session services, eight sites in a2a/, agents/functions.ts, agents/invocation_context.ts), so nothing insecure was left behind by changing it here. Three small comments below: the length of the hex assembly, a test that doesn't test what its name says, and an answer to the question you raised about the throw.

Correction to an earlier version of this comment: I originally wrote “six sites in a2a/” and 11 call sites total. The real counts are 8 in a2a/ (a2a_event.ts alone has five) and 13 overall. The conclusion is unchanged — all of them route through the helper this PR changes — but the numbers were wrong and the recount is what actually supports the claim.

One thing outside this PR's scope, while UUID generation is in view: code_executors/unsafe_local_code_executor.ts:53 builds a temp directory name with Math.random().toString(36) under os.tmpdir(). Predictable names in a world-writable directory are the classic pre-creation/symlink-squat hazard, and it is a closer relative of this PR than it looks. Not a request to change it here — worth its own issue. (events/event.ts:208 also uses Math.random, but only for an event id, which is not a security boundary.)

[AmaadMartin]

Nit. The byte-to-hex assembly is 19 lines doing what three can do.

    const bytes = new Uint8Array(16);
    globalThis.crypto.getRandomValues(bytes);
    // RFC 4122 section 4.4: version 4 in the high nibble of octet 6, variant
    // 10xx in the two high bits of octet 8.
    bytes[6] = (bytes[6] & 0x0f) | 0x40;
    bytes[8] = (bytes[8] & 0x3f) | 0x80;

    const hex: string[] = [];
    for (let i = 0; i < bytes.length; i++) {
      hex.push(bytes[i].toString(16).padStart(2, '0'));
    }

    return [
      hex.slice(0, 4).join(''),
      hex.slice(4, 6).join(''),
      hex.slice(6, 8).join(''),
      hex.slice(8, 10).join(''),
      hex.slice(10, 16).join(''),
    ].join('-');

    const bytes = globalThis.crypto.getRandomValues(new Uint8Array(16));
    // RFC 4122 section 4.4: version 4 in the high nibble of octet 6, variant
    // 10xx in the two high bits of octet 8.
    bytes[6] = (bytes[6] & 0x0f) | 0x40;
    bytes[8] = (bytes[8] & 0x3f) | 0x80;

    const hex = Array.from(bytes, (b) => b.toString(16).padStart(2, '0'));
    return [
      hex.slice(0, 4),
      hex.slice(4, 6),
      hex.slice(6, 8),
      hex.slice(8, 10),
      hex.slice(10, 16),
    ]
      .map((group) => group.join(''))
      .join('-');

Or, if you want it shorter still, join once and slice by hex-character offset, which reads closer to the 8-4-4-4-12 shape of a UUID than the doubled byte offsets do:

    const hex = Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join('');
    return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`;

Two things I checked before suggesting the first line: getRandomValues returns the array it was handed (per the Web Crypto spec, and your own stub on line 93 of the test file already returns it), so folding the allocation into the call is safe with your tests as written. And Array.from(uint8Array, fn) type-checks under the repo's lib: ["ES2022", "DOM"] — Uint8Array satisfies ArrayLike<number>, no downlevelIteration needed. The template literal in the second form is over 80 columns; prettier can't split template literals so it will leave it, which is why I put the array version first.

[AmaadMartin]

Nit, optional — this is me answering the question you raised in the description rather than asking for a change. Failing closed is the right call, and the blast radius is smaller than it looks, but it isn't quite zero.

Where the third branch is actually reachable:

• Browsers. core/build.js:11 sets the browser targets to chrome58, firefox57, safari11. None of those have crypto.randomUUID (Chrome 92+, Safari 15.4+), and all of them have getRandomValues, which predates randomUUID by about a decade. So branch 2 catches every browser you build for — the fallback is load-bearing, not a "just in case" path, and it is worth more than the secure-context case alone.
• Node. Per the Node docs for the crypto global, it was behind --experimental-global-webcrypto until v19 ("v19.0.0 — No longer behind --experimental-global-webcrypto CLI flag"). So on Node 18 and earlier with default flags there is no globalThis.crypto at all and this now throws where it previously returned a weak UUID — surfacing as a hard failure in createSession, AuthHandler.generateAuthUri, and every A2A message id, not as a degraded value.

Node 18 is EOL, so I don't think that argues for keeping a Math.random() path — I agree with removing it. But no package.json in this repo has an engines field, so nothing currently states that floor; the only signal is @types/node@^20. Worth declaring it in core/package.json so the throw is a documented contract instead of a runtime surprise:

  "engines": {
    "node": ">=20"
  },

I did not check whether release-please or the publish flow has an opinion about that field, so treat it as a suggestion rather than a verified-safe edit.

[AmaadMartin]

Nit. This test passes whichever branch runs, so it doesn't pin the one in its name.

    it('uses crypto.randomUUID when it is available', () => {
      expect(randomUUID()).toMatch(UUID_V4);
    });

UUID_V4 matches the getRandomValues output just as well — that is exactly what the next test asserts. So if crypto.randomUUID stopped being reached (the first if inverted, the branch reordered), this stays green. Stubbing both makes it discriminating:

    it('uses crypto.randomUUID when it is available', () => {
      setCrypto({
        randomUUID: () => '00000000-0000-4000-8000-000000000000',
        getRandomValues: () => {
          throw new Error('getRandomValues must not be called');
        },
      });

      expect(randomUUID()).toBe('00000000-0000-4000-8000-000000000000');
    });

That uses only setCrypto, so no new vi import. The trade-off is that it stops exercising the real platform randomUUID(); if you want to keep that coverage, keep the existing case as well and rename it to what it actually asserts — something like 'returns a valid v4 UUID on this runtime'.