registrystack / registrystack/registry-stack

evidence-client-node: type the config exclusivity rules the runtime enforces

Open
#1,064 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2
Forks
0
Avg merge
2h 55m
Merged PRs (30d)
130

Description

## What

In `crates/registry-evidence-client-node/client.d.ts`, `EvidenceClientConfig`
declares the two credential members as independent optionals:

```ts
/** Configure exactly one of `token` or `authorization`. */
token?: EvidenceTokenConfig
authorization?: { exchange: EvidenceExchangeAuthorizationConfig }
```

The doc comment states the rule the runtime enforces, but the type does not.
A caller can pass both, or neither, and type-check cleanly.

## Why it is not urgent

The runtime already refuses both shapes, with a message that names the rule:
"configure exactly one of `token` or `authorization`". So this is a typing
ergonomics gap, not a correctness or security gap: the failure is a clear
run-time error rather than a silent wrong behaviour. Nothing is accepted that
should be refused.

## What to do

Give the two members an exclusive union the way
`EvidenceExchangeAuthorizationConfig` already does for its `firstParty` /
`remote` pair, so TypeScript reports the mistake where the caller writes it:

```ts
export type EvidenceClientConfig = {
baseUrl: string
// ... the shared members ...
} & (
| { token: EvidenceTokenConfig, authorization?: never }
| { token?: never, authorization: { exchange: EvidenceExchangeAuthorizationConfig } }
)
```

Points to settle while doing it:

- `EvidenceClientConfig` is currently an `interface`. An intersection with a
union has to be a `type`, which is a breaking change for any caller that
writes `interface Mine extends EvidenceClientConfig`. Check whether that
matters for a pre-1.0 binding before choosing.
- The same shape exists in the generated facade copy at
`crates/registry-stack-client-node/evidence/client.d.ts`. That file is
produced by `python3 release/scripts/sync-registry-client-node.py` and must
be regenerated, never hand-edited.
- `crates/registry-evidence-client-node/__test__/drift.test.js` reads the
interface body by name to compare it against the Rust reader's fields.
Turning the declaration into a `type` will need that helper taught the new
shape, otherwise the existing drift tests stop covering the config.
- The Python binding does not type config dict shapes at all, so there is
nothing to mirror on that side.

## Origin

Raised by the Codex review on #1063 and deferred there deliberately: the
run-time refusal is correct today, and the fix touches the declaration form of
the public config type plus its drift tests, which is more than that PR should
carry.

---

## Second item: couple `grantId` to the selected assertion source

Same file, same class of gap, and worth doing in the same pass.

`EvidenceExchangeAuthorizationConfig` already splits `firstParty` and `remote`
into an exclusive union, but it keeps `context` in the shared half, so
`context.grantId` is optional for both arms. The runtime pairs the two:

- `exchange_config.rs` chooses the context kind from `grantId` alone: present
means a grant context, absent means a first-party context.
- `ExchangeAuthorization::first_party` refuses a grant context ("a first-party
context cannot contain a grant").
- `ExchangeAuthorization::from_authority` refuses a context without one ("an
authority source requires a grant context").

So `firstParty` carrying a `context.grantId`, and `remote` without one, both
type-check and then always fail construction.

Fix: move `context` into each union arm and state the pairing there.

```ts
| { firstParty: {...}, remote?: never,
context: EvidenceExchangeContext & { grantId?: never } }
| { firstParty?: never, remote: EvidenceExchangeRemoteSourceConfig,
context: EvidenceExchangeContext & { grantId: string } }
```

Not urgent for the same reason as the item above: both shapes are refused at
construction with a message naming the rule, so nothing is accepted that
should be refused. The drift test helper that reads `Context`'s fields out of
`exchange_config.rs` will need to follow the declaration into the arms.

Raised by the Codex review on #1063 and deferred there with the first item.

Contributor guide

Open the contributing guide

Research direction

Start with crates/registry-evidence-client-node/client.d.ts and compare its config declarations with EvidenceExchangeAuthorizationConfig. Read crates/registry-evidence-client-node/__test__/drift.test.js, the Rust exchange_config.rs references, and the sync script before changing the declaration form. Done means both exclusivity rules are represented, the generated facade is regenerated, and the drift tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, python, rust, typescript
Domain
api, developer-experience, testing
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.