Enhance HTTP security and YUCP trust bundle handling - #18

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/skills/impeccable/scripts/cleanup-deprecated.mjs:
- Around line 8-9: Update the usage comment so it's copy-pasteable by replacing
the templated `{{scripts_path}}` with the actual relative path to this script;
change the example line `node {{scripts_path}}/cleanup-deprecated.mjs` to `node
.agents/skills/impeccable/scripts/cleanup-deprecated.mjs` (edit the Usage
comment that surrounds that line in cleanup-deprecated.mjs).
- Around line 44-47: The root detection and CLI guard are POSIX-only; in
findProjectRoot replace the hard-coded `{ root: '/' }` usage with
path.parse(dir).root (ensure path.parse is imported) so root detection works on
Windows drives, and replace any new URL(import.meta.url).pathname comparisons
(used in the CLI guard around import.meta.url at/near line 200) with
fileURLToPath(import.meta.url) (ensure fileURLToPath is imported from 'url') so
path comparisons are platform-correct.

In `@apps/api/src/lib/httpSecurity.ts`:
- Around line 15-18: The current content-type check only matches literal
"application/json" so vendor or structured JSON types like
"application/problem+json" are missed; update the condition that computes
contentType and the if-statement (the contentType variable and the headers
check) to treat any media type that is either "application/json" or
ends/contains "+json" as JSON (e.g., change the if to check
contentType.includes('application/json') || contentType.includes('+json')) and
then set Cache-Control: no-store via headers.set when that condition is true and
Cache-Control is not already present.

In `@apps/bot/src/lib/convexActor.ts`:
- Around line 95-102: The predicate in shouldAttachActor currently returns true
even when callers explicitly provided args.actor, causing auto-attachment to
overwrite it; update shouldAttachActor (and keep using describeFunctionReference
and isApiActorProtectedFunction) to only auto-attach when the args object does
NOT contain an actor property (e.g., after verifying !!args && typeof args ===
'object' && !Array.isArray(args), add a check like !('actor' in (args as
Record<string, unknown>)) or use Object.prototype.hasOwnProperty.call to ensure
explicit actor bindings are preserved).

In `@convex/http.ts`:
- Around line 1257-1267: The code currently always verifies bearer certs with
signingRoot.publicKeyBase64 (from getPinnedSigningRoot), which breaks validation
for certs issued under previous roots; change the flow in the /v1/signatures
handling so you first parse/extract the cert envelope (e.g., get its kid)
without enforcing verification, then look up the correct public key from the
active trust bundle (or pinned roots) keyed by that kid, and finally call
parseBearerCert or the verification step with the selected public key; update
the logic that calls getPinnedSigningRoot and parseBearerCert so verification
uses the key matched to the cert's kid rather than the current root's
publicKeyBase64.
- Around line 210-225: getClientAddress is currently trusting client-controlled
headers (x-real-ip, x-forwarded-for) which allows IP spoofing; update
getClientAddress to only accept a platform/trusted-proxy supplied source (e.g.,
keep cf-connecting-ip) and do not use x-real-ip or x-forwarded-for unless you
first verify the request came through a trusted proxy (via config/allowlist or a
platform-provided flag). Replace the fallback logic in getClientAddress so it
returns cf-connecting-ip when present, otherwise uses a server-side/trusted
connection source (e.g., the real remote address supplied by the runtime) or
returns 'unknown' — remove unconditional parsing of x-real-ip and
x-forwarded-for to prevent attacker-controlled spoofing.

In `@convex/migrations.ts`:
- Around line 210-212: The code assigns licenseKeyEncrypted by falling back to
awaiting encryptPii(doc.licenseKey, PII_PURPOSES.forensicsLicenseKey) even when
doc.licenseKey may be undefined; update the assignment for licenseKeyEncrypted
(and references to doc.licenseKeyEncrypted and encryptPii) to guard the async
call so encryptPii is only invoked when doc.licenseKey is non-null/defined (e.g.
use a conditional that returns undefined when doc.licenseKey is falsy rather
than calling encryptPii).

In `@convex/testHelpers.ts`:
- Around line 108-127: mergeActorArg currently always overwrites args.actor
which hides tests that intentionally pass a custom/invalid actor; change
mergeActorArg to only add the actor when args is not an object (same existing
branch) or when args is an object but does not already have an "actor" property,
leaving an explicit actor intact; ensure shouldAttachActor logic (function
shouldAttachActor and call sites that use it) remains consistent with the new
behavior so that actor injection only happens when there's no explicit actor
present.

In `@convex/yucpCertificates.ts`:
- Around line 316-317: The code currently only reads process.env.YUCP_KEY_ID and
ignores the legacy process.env.YUCP_ROOT_KEY_ID, breaking backwards
compatibility; update the environment lookup (where YUCP_KEY_ID is read in
convex/yucpCertificates.ts — the variable or constant that holds the pinned key
id) to fallback to process.env.YUCP_ROOT_KEY_ID when YUCP_KEY_ID is undefined
(e.g., const yucpKeyId = process.env.YUCP_KEY_ID ??
process.env.YUCP_ROOT_KEY_ID), and ensure any places using that variable (or
function that returns the key id) continue to work unchanged; mirror the same
pattern used in convex/http.ts around the YUCP_* env handling.

In `@convex/yucpLicenses.realtest.ts`:
- Around line 19-41: The test mutates process.env keys and globalThis.fetch in
beforeEach but only resets pinned roots; capture originals (e.g., save
originalFetch and original values of YUCP_ROOT_PRIVATE_KEY,
YUCP_ROOT_PUBLIC_KEY, YUCP_ROOT_KEY_ID, POLAR_ACCESS_TOKEN,
POLAR_WEBHOOK_SECRET, YUCP_COUPLING_SERVICE_BASE_URL,
YUCP_COUPLING_SERVICE_SHARED_SECRET) and restore them in afterEach alongside
calling setPinnedYucpRootsForTests(null), ensuring globalThis.fetch and all
modified process.env entries are returned to their prior values (use the same
variable names like originalFetch and the YUCP_* env names to locate where to
store/restore).

In `@packages/shared/src/yucpTrust.ts`:
- Around line 139-163: The current try/catch in the trust-bundle parsing
silently falls back to getPinnedYucpRoots() on any parse/validation error;
change it to "fail closed" by rethrowing (or returning an explicit error) when
an explicit bundle was provided. In the catch block around JSON.parse(...) /
validation (the block that now returns {version:1, roots:
getPinnedYucpRoots()}), detect whether the caller provided a non-empty json
input (or process.env.YUCP_TRUST_BUNDLE_JSON is set) and if so throw a new Error
including the original error message; only fall back to getPinnedYucpRoots()
when no explicit bundle was supplied. Ensure the thrown error references the
parsing/validation failure so callers can surface the misconfiguration.
- Around line 46-52: The JWK `x` is being written and read using standard base64
instead of base64url, causing non-compliant OKP/Ed25519 JWKs and failed parsing;
update normalizeTrustJwk (which constructs YucpTrustJwk from YucpPinnedRoot) to
emit `x` encoded as base64url (RFC 8037), update the code path that reads
`record.x` to decode base64url when reconstructing keys, and stop silently
swallowing parse errors during bundle parsing (surface or propagate the error
instead of falling back silently to builtin roots) so invalid encodings are
detected; reference normalizeTrustJwk, YucpPinnedRoot, YucpTrustJwk and the
record.x parsing logic when making the changes.

---

Outside diff comments:
In `@apps/bot/src/commands/licenseVerify.ts`:
- Around line 577-609: The call to getRequiredBotActorBinding() can throw but is
executed outside the try-catch that handles ensureSubjectForDiscord, creating an
unhandled error path; move or duplicate getRequiredBotActorBinding() invocation
into the same try block (or wrap it in its own try) so any thrown error is
caught and handled the same way as errors from
convex.mutation(api.subjects.ensureSubjectForDiscord), and ensure you still
assign subjectId from ensureResult and run the same error branch (logging via
logger.error and sending the same interaction.editReply paths) when
getRequiredBotActorBinding() fails.
- Around line 461-481: getRequiredBotActorBinding() can throw before the
existing try-catch around convex.mutation, so wrap the actor retrieval in the
same error-handling path: move the call to getRequiredBotActorBinding() into the
try block (or add a short try/catch around it) so any errors are caught, logged
with logger.error (including the error and discordUserId), and the user is
notified via interaction.editReply with the same friendly failure message;
ensure you still call convex.mutation(api.subjects.ensureSubjectForDiscord, {
actor, ... }) only after actor is successfully obtained and preserve the
existing subjectId check and error handling.

In `@convex/downloads.ts`:
- Around line 154-175: After fetching guildLink with ctx.db.get and checking
ownership via requireOwnedDocument, also verify that the retrieved guildLink's
guildId matches args.guildId; if they differ, reject the request (throw an error
or return a 400/403) before calling ctx.db.insert to create the download route.
Update the logic around guildLink/guildLinkId validation in downloads.ts (the
code that calls requireOwnedDocument and then inserts into 'download_routes') to
perform this additional equality check and fail fast when guildId and
guildLink.guildId are inconsistent.
- Around line 311-333: The insert is using routing metadata from args which
allows inconsistent/malicious routing; change the
ctx.db.insert('download_artifacts', ...) call to source all route-derived fields
from the fetched route instead of args: use route.guildId,
route.sourceChannelId, route.sourceMessageId, route.sourceMessageUrl,
route.sourceAuthorId, route.archiveChannelId, route.archiveMessageId,
route.archiveThreadId, route.sourceRelayMessageId, route.sourceDeliveryMode and
use route.roleLogic and a de-duplicated route.requiredRoleIds (e.g. [...new
Set(route.requiredRoleIds || [])]); keep authUserId, files, status,
createdAt/updatedAt etc from args/context as appropriate and leave
requireOwnedDocument(route, args.authUserId) in place.

In `@convex/entitlements.ts`:
- Around line 1387-1401: Replace the delegated-creator authorization in this
scheduler mutation: remove the call to requireDelegatedAuthUserActor(args.actor,
args.authUserId) in the mutation handler and enforce a service-scoped actor
check instead (use or add a helper such as requireServiceActor(args.actor) or
requireSystemActor(args.actor) and call that before proceeding); keep the
requireApiSecret(args.apiSecret) check and continue using Date.now() as before,
and return the same payload shape (success, expiredCount, entitlementIds).

In `@convex/http.ts`:
- Around line 1362-1414: The protected-assets size check currently runs after
performing mutations (internal.packageRegistry.registerPackage and
internal.signingLog.writeEntry), risking partial commits; move the guard that
checks body.protectedAssets.length against MAX_PROTECTED_ASSETS_PER_REQUEST to
run before calling registerPackage or signingLog.writeEntry (i.e., at the top of
this handler), and return the same errorResponse(...) 400 immediately if
exceeded so no mutations are executed when the payload is oversized.
- Around line 476-507: In buildManifestCertificateChain, the issuerKeyId is
incorrectly taken from envelope.signature.keyId (rootKeyId) instead of the
actual signer’s key id; after calling
signPackageCertificateData(publisherCertificateBase, rootPrivateKey) capture the
signature metadata and set issuerKeyId on publisherCertificateBase (and the
returned publisher cert) to the signer's key id (e.g., the keyId from the
publisherSignature/result of signPackageCertificateData) so the chain records
the actual key that signed it (update buildManifestCertificateChain,
publisherCertificateBase.issuerKeyId, and the returned certificate object
accordingly).

In `@convex/yucpLicenses.ts`:
- Around line 1741-1760: The verification-only paths (e.g.,
redeemProtectedMaterializationGrant and issueCouplingJob) are calling
getPinnedSigningRoot(...) which currently requires the private root key; change
these to load only the pinned trust bundle/public roots so verification does not
depend on YUCP_ROOT_PRIVATE_KEY. Specifically, replace calls to
getPinnedSigningRoot(process.env.YUCP_ROOT_KEY_ID ?? null) in the shown block
(and the similar calls around lines 1973-1976) with a function or flag that
returns only the public trust bundle/pinned roots (e.g., getPinnedTrustBundle or
getPinnedSigningRoot({publicOnly:true})), and ensure downstream verification
functions (verifyProtectedUnlockJwtAgainstPinnedRoots) still receive the same
issuer/trust data but never trigger private-key loading or signing logic.

---

Nitpick comments:
In `@apps/api/src/routes/couplingLicenses.ts`:
- Around line 280-287: In issueCouplingJob, move the pinned-root availability
check (the try/catch calling getPinnedSigningRoot) to occur before any call to
resolveRuntimeArtifact so the function fails fast on configuration issues;
mirror the ordering used in issueRuntimePackageToken by relocating the existing
try { await getPinnedSigningRoot(); } catch (...) block to precede the
resolveRuntimeArtifact(...) invocation and return the same errorResponse on
failure.

In `@apps/api/src/routes/publicV2/verification-intents.test.ts`:
- Around line 28-31: Add an assertion in the verification-intents.test.ts that
the mocked actorBinding object is forwarded as part of the Convex action args:
after the test triggers the action, inspect observedCalls[0]?.[1] (the Convex
action's args) and assert it includes the actorBinding payload and signature you
set (e.g., { actorBinding: { payload: 'test-payload', signature:
'test-signature' } }). This ensures the test verifies the pass-through contract
for the actorBinding when calling the Convex action.

In `@apps/api/src/routes/publicV2/webhooks.test.ts`:
- Around line 47-50: Test coverage missing an assertion that the mocked gateway
receives the actorBinding object; update at least one webhook route test that
triggers queryMock or mutationMock to assert the mock was called with an args
object containing actorBinding: { payload: 'test-payload', signature:
'test-signature' }. Locate the test that invokes queryMock or mutationMock in
webhooks.test.ts and add an assertion on the call args (e.g.,
expect(queryMock).toHaveBeenCalledWith(expect.objectContaining({ actorBinding: {
payload: 'test-payload', signature: 'test-signature' } })) or the equivalent for
mutationMock) so the actorBinding pass-through contract is verified.

In `@apps/bot/src/commands/downloads.ts`:
- Around line 625-631: The code repeatedly awaits getRequiredBotActorBinding
inside handleDownloadsAutofixPrompt and handleDownloadsAutofixRun (and similar
blocks at the 668-674 diff), causing duplicate awaits for the same actor;
refactor by extracting actor retrieval out of the inner flows — either fetch
once at the top-level of the command handler and pass the actor into
handleDownloadsAutofixPrompt/handleDownloadsAutofixRun as a parameter, or
introduce a short-lived cached variable (e.g., let actor = await
getRequiredBotActorBinding()) used by both functions; update the calls and
signatures for these handlers (handleDownloadsAutofixPrompt,
handleDownloadsAutofixRun) accordingly to avoid re-awaiting the same binding.

In `@convex/backgroundSync.ts`:
- Around line 446-449: Extract a small helper function (e.g.,
getBackgroundSyncActor or createBackgroundSyncActor) that encapsulates the
createServiceActorBinding call with service: 'background-sync' and scopes:
['creator:delegate'], then replace the duplicated inline blocks that assign to
actor with calls to this helper; update both occurrences (the one that assigns
const actor = await createServiceActorBinding({...}) and the other duplicate) so
they await the new helper instead of repeating the binding object.

In `@convex/httpCertificates.contract.test.ts`:
- Around line 19-22: The test is too tied to an implementation-local variable
name `raw`; remove the assertion that expects "const raw = err instanceof Error
? err.message : ''" and instead assert only the observable contract: that the
source contains the generic errorResponse("Certificate issuance failed", 500)
and does not include any return that exposes the underlying error (e.g. strings
containing "err.message", "String(err)" or interpolation of err). Update the
test in the "hides raw issuance errors behind a generic 500 response" spec to
drop the `raw` containment check and keep/expand the negative containment checks
against any direct usage of err in errorResponse while still verifying the
generic 500 response via httpSource and the existing literals.

In `@convex/lib/licenseSubjectLink.ts`:
- Around line 3-14: The LicenseSubjectLinkInput type currently includes optional
fields licenseKey and purchaserEmail which are not persisted; remove these two
properties from the LicenseSubjectLinkInput type declaration (and any duplicate
declaration around the other occurrence) or mark them explicitly deprecated with
a clear comment/JSDoc if you need temporary backward compatibility, and update
any related references or callers to stop passing plaintext PII (search for the
symbol LicenseSubjectLinkInput and amend both declarations at the other
occurrence noted). Ensure tests/types compile after removal and that
insert/update code paths (e.g., functions handling license subject links)
continue to only persist the intended fields.

In `@convex/lib/trustedOrigins.ts`:
- Around line 6-19: Extract the LOCAL_TRUSTED_BROWSER_ORIGINS constant into the
shared package and import it here to avoid duplication: move the array from
convex/lib/trustedOrigins.ts into packages/shared (export as
LOCAL_TRUSTED_BROWSER_ORIGINS from e.g. authOrigins.ts), update
convex/lib/trustedOrigins.ts to import and re-export or reference that exported
symbol instead of redefining it, and update any other files (like
packages/shared/src/authOrigins.ts) to consume the single exported constant so
ports/origins are maintained in one place.

In `@convex/yucpTrustBundle.contract.test.ts`:
- Around line 8-18: Replace fragile source-string assertions in the test with
behavior-level assertions that invoke the /v1/keys route handler (or the
exported function that builds its response) and assert the response JSON shape:
verify the response includes a trustBundle field and the legacy JWK set (i.e.,
presence of getConfiguredYucpJwkSet output), and that signing is delegated via
signYucpTrustBundleJwt; additionally assert the issuer value equals the
configured public API base URL by calling resolveConfiguredPublicApiBaseUrl()
(and assert it is not requestUrl.origin). Locate references to httpSource, the
test cases that mention '/v1/keys', signYucpTrustBundleJwt,
getConfiguredYucpJwkSet, and resolveConfiguredPublicApiBaseUrl to replace string
containment checks with direct handler invocation and JSON shape/semantic
assertions.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/skills/impeccable/scripts/cleanup-deprecated.mjs:
- Around line 48-60: The loop stops when dir === root without checking root for
repo markers, so modify the exit logic to check the root directory once before
falling back: after the while loop (which currently checks existsSync(join(dir,
...)) for intermediate dirs), add a check using existsSync(join(root,
'package.json')), existsSync(join(root, '.git')), or existsSync(join(root,
'skills-lock.json')) and return root if any exist; otherwise continue to return
resolve(startDir). Reference symbols: dir, root, startDir, existsSync, join,
resolve.

In `@apps/bot/src/lib/convexActor.ts`:
- Around line 20-73: The cachedDefaultBotActor is only time-keyed so rotating
INTERNAL_SERVICE_AUTH_SECRET can leave callers using an old binding; modify
getDefaultBotActorBinding to include the current secret in the cache key (e.g.,
store secret or its hash alongside cachedDefaultBotActor) and
invalidate/recreate the binding when process.env.INTERNAL_SERVICE_AUTH_SECRET
changes before returning the cached binding; update cachedDefaultBotActor to
hold the secret value and check it against the current secret prior to returning
the cached binding (still respect the existing expiresAt logic and reuse
createServiceApiActor/createApiActorBinding when recreating).

In `@convex/downloads.ts`:
- Around line 154-160: Assign the return value of requireOwnedDocument to the
local variables so TypeScript knows they are non-null: replace the discarded
call with e.g. guildLink = requireOwnedDocument(guildLink, args.authUserId,
'Unauthorized: caller does not own this guild link') in the downloads handler,
and do the same in the createArtifact handler for route (route =
requireOwnedDocument(route, args.authUserId, '...')). This ensures guildLink and
route are narrowed non-nullable before you access their properties.
- Around line 314-331: The code calls requireOwnedDocument(route,
args.authUserId) but ignores its return, leaving the local variable route
nullable and causing TS errors when accessing its properties in the insert call;
fix by using the narrowed/non-null return from requireOwnedDocument (e.g., const
ownedRoute = requireOwnedDocument(route, args.authUserId) or reassign route =
requireOwnedDocument(route, args.authUserId)) and then reference ownedRoute (or
the reassigned route) for properties used in the ctx.db.insert into
'download_artifacts' so TypeScript recognizes the object as non-null (affecting
uses of route.guildId, route.sourceChannelId, route.sourceMessageId,
route.sourceMessageUrl, route.sourceAuthorId, route.archiveChannelId,
route.archiveMessageId, route.archiveThreadId, route.sourceRelayMessageId,
route.sourceDeliveryMode, route.requiredRoleIds, route.roleLogic).

In `@convex/http.ts`:
- Around line 235-237: The rate-limit key construction uses the
attacker-controlled options.identity as the primary identifier (in principal and
key), allowing callers to evade limits; change the key so the stable caller
identifier (e.g., getClientAddress(request) and any authenticated subject, not
options.identity alone) is the main bucket and include resource IDs only as a
secondary dimension. Concretely, update the logic around principal and key (see
principal, key, getClientAddress, sha256HexHttp, and options.identity) to build
key = `${scope}:${await sha256HexHttp(stableCallerId)}` (where stableCallerId is
getClientAddress(request) plus any verified auth subject) and, if needed, append
a resource-specific suffix instead of using options.identity as the primary
hashed value.

In `@convex/protectedMaterializationGrant.realtest.ts`:
- Around line 28-35: The test fixture must isolate YUCP_TRUST_BUNDLE_JSON so
setPinnedYucpRootsForTests() runs deterministically; update the ENV_KEYS
constant or the fixture setup to include 'YUCP_TRUST_BUNDLE_JSON' (alongside
YUCP_ROOT_PRIVATE_KEY, YUCP_ROOT_PUBLIC_KEY, etc.) so the env is controlled, or
explicitly delete process.env.YUCP_TRUST_BUNDLE_JSON before calling
setPinnedYucpRootsForTests(); locate the ENV_KEYS declaration and the test setup
that invokes setPinnedYucpRootsForTests() and apply one of these changes to
ensure the pinned root configured in the test cannot be overridden by an
external environment variable.

In `@convex/testHelpers.ts`:
- Around line 48-105: The cache reuse in getTestActorBinding can return a
binding signed with a different INTERNAL_SERVICE_AUTH_SECRET; update the
cachedTestActor structure to include the secret used (e.g., add a secret: string
field) and change the cache check to ensure cachedTestActor.expiresAt > now +
30_000 AND cachedTestActor.secret === secret before returning
cachedTestActor.binding; if the secret differs, create a new actor/binding via
createServiceApiActor and createApiActorBinding, store the new secret on
cachedTestActor along with binding and expiresAt, and then return the new
binding.

In `@convex/yucpCertificates.contract.test.ts`:
- Around line 7-10: The contract test "supports both legacy and current root key
id env names" currently expects YUCP_KEY_ID to take precedence; update the
assertion so the new root-key precedence is enforced by expecting
process.env.YUCP_ROOT_KEY_ID ?? process.env.YUCP_KEY_ID ?? null instead (so
YUCP_ROOT_KEY_ID wins when both are present), matching the precedence used in
convex/http.contract.test.ts and ensuring both surfaces select the same root.

In `@convex/yucpLicenses.realtest.ts`:
- Around line 15-23: The tests currently enumerate ENV_KEYS but omit
YUCP_TRUST_BUNDLE_JSON, so the verifier may pick up an existing
process.env.YUCP_TRUST_BUNDLE_JSON; update the ENV_KEYS constant to include
'YUCP_TRUST_BUNDLE_JSON' and ensure the test setup/teardown that iterates
ENV_KEYS (used to clear and restore env vars) also clears/restores this
variable—locate the ENV_KEYS array and the corresponding setup/teardown code in
yucpLicenses.realtest.ts and add the new key so the bundle is always reset for
these tests.

In `@convex/yucpLicenses.ts`:
- Line 1232: The call to getPinnedSigningRoot is only using
process.env.YUCP_ROOT_KEY_ID, dropping the legacy YUCP_KEY_ID fallback; update
each call to pass process.env.YUCP_ROOT_KEY_ID ?? process.env.YUCP_KEY_ID ??
null so the legacy env var is honored (e.g., update the occurrences that call
getPinnedSigningRoot and any similar signing-path calls to use this combined
fallback).
- Around line 1036-1049: The current fallback always calls encryptPii which can
be passed undefined (in the licenseKeyEncrypted assignment), so change the logic
for licenseKeyEncrypted to: use args.licenseKeyEncrypted if present, else if
args.licenseKey is present call encryptPii(args.licenseKey,
PII_PURPOSES.forensicsLicenseKey), otherwise leave licenseKeyEncrypted
undefined/null; update the assignment that computes licenseKeyEncrypted and
ensure upsertLicenseSubjectLink is passed that possibly-undefined value (do not
call encryptPii with undefined). This affects the licenseKeyEncrypted assignment
and the encryptPii call in the surrounding block.

In `@packages/shared/src/apiActor.test.ts`:
- Line 21: Reformat the failing test assertion so the expectation uses the
promise-resolves style in one properly spaced expression: call
verifyApiActorBinding(binding, 'test-secret', now + 1_000) inside await
expect(...).resolves.toEqual(actor) (ensure the parentheses, spacing and
semicolon match the shared formatter's style for the expression and trailing
semicolon).

In `@packages/shared/src/yucpTrust.ts`:
- Around line 164-169: The current mapping silently drops invalid entries by
filtering out nulls from the parseTrustedRootCandidate results; change this to
fail-fast by detecting any nulls and throwing instead. Update the code around
the keys.map(... parseTrustedRootCandidate ...) so you collect the raw mapped
results, check for any item === null (optionally recording the index or original
entry), and if any are present throw a descriptive Error like 'invalid trust
bundle: contains invalid entries' (or include offending entries); only when all
entries are non-null convert/cast the array to YucpTrustedRoot[] and proceed.

---

Outside diff comments:
In `@convex/http.ts`:
- Around line 1743-1748: The nonce is being consumed before resolving the
signing root which can convert transient signing-root errors into permanent
client failures; call getPinnedSigningRoot(process.env.YUCP_ROOT_KEY_ID ?? null)
and await it first to ensure the signing root is valid, then invoke
ctx.runMutation(internal.yucpLicenses.checkAndConsumeNonce, { nonce }) to
consume the nonce and proceed to issue the JWT (update the order around
getPinnedSigningRoot and checkAndConsumeNonce).

---

Nitpick comments:
In `@convex/downloads.ts`:
- Around line 283-299: The public argument schema for the createArtifact
mutation still requires route-derived fields (guildId, sourceChannelId,
archiveChannelId, requiredRoleIds, roleLogic) even though createArtifact
persists those from the saved route; update the handler by removing these fields
from the public args contract (the argument validator/schema used by
createArtifact in convex/downloads.ts) so callers don't need to supply them, or
alternatively add explicit validation inside createArtifact to reject requests
where any of those caller-supplied values conflict with the saved route values
(compare incoming
guildId/sourceChannelId/archiveChannelId/requiredRoleIds/roleLogic against the
route and throw on mismatch).