fix(management): close the review findings in the pipe contract and the log's redaction #51

2026-11-11T06:22:35Z

james.maes commented

2026-11-11 06:22:35 +00:00

Fixes from the retrospective review of the version 2 management pipe contract and of the service log's redaction. The contract is types only: nothing uses it yet, so no installed behaviour changes except what the log hides.

The contract (src/Qqq.PrintAgent.Windows/Management/)

An error's message is bounded and printable however the error is made; a failure's details are tied to its code, and a code outside the table is refused when the service builds an error.
The four envelope types refuse the JSON serializer, for reading and writing; the contract writes and reads them itself. The wire text is unchanged: every golden file but two is byte-identical.
Requests and replies can only be built through functions that check the operation, the server selector and the version.
The parser never throws. A malformed text escape is answered by position (a malformed operation, a malformed server, an incomplete reply, or not JSON); a 340-case table pins it.
Text on the wire has its own rule: an exact cut with no mark, the same result when applied twice, a missing identity value left out.
One server-name rule and one server-address rule, for the service and the wizard to share. A name is 1 to 64 units, shows something, and holds no square bracket; an address is refused when its host has no ASCII name or is an IPv6 literal with a zone.
servers.add and trust.set carry the sign-in provider the operator was shown by the check. The service refuses with the new code provider_changed when the provider it reads is not that one, including when it reads one where none was shown. Golden files servers.add.request.json and trust.set.request.json gain providerOrigin.
printers.discover is marked as the one read that joins a run in progress.

The log's redaction

Every field of the product's own request and response bodies that carries a secret is now hidden as a JSON member, as name=value, and percent-encoded: the enrollment code, device token, session identifiers and token, the exchanged code beside its verifier, the code verifier in every spelling, and the ID token hint.
An Authorization or Proxy-Authorization header loses its value whatever the scheme.
A nonce is deliberately not hidden; the test says why. What is still not recognised, and what is hidden although it is not a secret, is pinned by tests and listed in audit rows W40 and W41.

Verification

Head 3779c51 on release/2.0 at db45420.
build/Verify.ps1 passed twice on that head: 1,174 Core, 2,104 Windows runtime and command line, 360 presentation tests, none failed; Core 100% of 2,311 lines; Windows 88.72%.

Review (independent reviewer, three passes)

808f459: 0 blockers, 2 should-fix, 12 nits. Every rule the branch claims held against every route the reviewer tried; the should-fix items were the product's own spelling of the code verifier, and an older parser crash.
de45164: 0 blockers, 0 should-fix, 1 nit. 100,656 inputs through the parser, none threw; the list of secret-carrying fields checked against the code.
3779c51, the head merged: Verdict on 3779c51: 0 blockers, 0 should-fix, 1 nit. (a message over 8,192 units whose verifier name falls after the cut keeps its code; nothing logs that body; to be recorded in the audit).
de45164 was moved onto db45420 as 89b56be: no code or test file conflicted and the diff over src, tests and build is line for line the same; the three audit rows this change adds were renumbered W74 to W76 because pull request 50 took W66 to W73.

Left open, in the audit

W35, W36, W38, W39, W41, W56 partly closed, with what remains in each row.
W74 to W76, C26, C27: residue of the envelope change; a name may begin with a hyphen (for the command-line step); the shown text of a zoned origin drops the zone; characters that draw nothing can still be a whole error message or identity value.

🤖 Generated with Claude Code

Fixes from the retrospective review of the version 2 management pipe contract and of the service log's redaction. The contract is types only: nothing uses it yet, so no installed behaviour changes except what the log hides. **The contract (`src/Qqq.PrintAgent.Windows/Management/`)** - An error's message is bounded and printable however the error is made; a failure's details are tied to its code, and a code outside the table is refused when the service builds an error. - The four envelope types refuse the JSON serializer, for reading and writing; the contract writes and reads them itself. The wire text is unchanged: every golden file but two is byte-identical. - Requests and replies can only be built through functions that check the operation, the server selector and the version. - The parser never throws. A malformed text escape is answered by position (a malformed operation, a malformed server, an incomplete reply, or not JSON); a 340-case table pins it. - Text on the wire has its own rule: an exact cut with no mark, the same result when applied twice, a missing identity value left out. - One server-name rule and one server-address rule, for the service and the wizard to share. A name is 1 to 64 units, shows something, and holds no square bracket; an address is refused when its host has no ASCII name or is an IPv6 literal with a zone. - `servers.add` and `trust.set` carry the sign-in provider the operator was shown by the check. The service refuses with the new code `provider_changed` when the provider it reads is not that one, including when it reads one where none was shown. Golden files `servers.add.request.json` and `trust.set.request.json` gain `providerOrigin`. - `printers.discover` is marked as the one read that joins a run in progress. **The log's redaction** - Every field of the product's own request and response bodies that carries a secret is now hidden as a JSON member, as `name=value`, and percent-encoded: the enrollment code, device token, session identifiers and token, the exchanged code beside its verifier, the code verifier in every spelling, and the ID token hint. - An `Authorization` or `Proxy-Authorization` header loses its value whatever the scheme. - A nonce is deliberately not hidden; the test says why. What is still not recognised, and what is hidden although it is not a secret, is pinned by tests and listed in audit rows W40 and W41. **Verification** - Head `3779c51` on `release/2.0` at `db45420`. - `build/Verify.ps1` passed twice on that head: 1,174 Core, 2,104 Windows runtime and command line, 360 presentation tests, none failed; Core 100% of 2,311 lines; Windows 88.72%. **Review (independent reviewer, three passes)** - `808f459`: `0 blockers, 2 should-fix, 12 nits`. Every rule the branch claims held against every route the reviewer tried; the should-fix items were the product's own spelling of the code verifier, and an older parser crash. - `de45164`: `0 blockers, 0 should-fix, 1 nit`. 100,656 inputs through the parser, none threw; the list of secret-carrying fields checked against the code. - `3779c51`, the head merged: `Verdict on 3779c51: 0 blockers, 0 should-fix, 1 nit.` (a message over 8,192 units whose verifier name falls after the cut keeps its code; nothing logs that body; to be recorded in the audit). - `de45164` was moved onto `db45420` as `89b56be`: no code or test file conflicted and the diff over `src`, `tests` and `build` is line for line the same; the three audit rows this change adds were renumbered W74 to W76 because pull request 50 took W66 to W73. **Left open, in the audit** - W35, W36, W38, W39, W41, W56 partly closed, with what remains in each row. - W74 to W76, C26, C27: residue of the envelope change; a name may begin with a hyphen (for the command-line step); the shown text of a zoned origin drops the zone; characters that draw nothing can still be a whole error message or identity value. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 30 commits 2026-11-11 06:22:35 +00:00

fix(management): bound and clean an error's message in the error itself 9e973ae922

The printable form and the bound of an error's message were applied by the
two ManagementReply.Failure factories and nowhere else, so an error built
through the public record constructors or a `with` expression, and the
refusal ManagementOperations.Check returns, were written as given, and a
client read back whatever the wire carried (audit W31, review A finding 1).

ManagementError now has one constructor, which makes the message printable
and cuts it, and no member that can be set afterwards: a `with` expression
can only copy it. The reply factories and the parser build their errors
through that constructor, so the rule holds for every error and for every
reply that carries one, written or read. ManagementReply.Failure gains an
overload that takes an existing error, so the dispatcher can send the
routing refusal without building a reply member by member.

The function applied is still Core's; its semantics for wire text (the
"(missing)" stand-in, the mark after a cut) are finding 4 and change in a
later commit of this series.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): tie a failure's details to its code and refuse an unknown code 4aeb761ed9

A failure could be built with any details object for any code and with any
text as a code: `busy` with details, `identity_changed` with an anonymous
object whose unbounded values a client then read as the typed details, a
code with a line break in it (audit W32, review A finding 2).

The service now has two ways to build an error and both check the table.
The constructor takes a code that carries no details; WithDetails takes a
code that carries some, together with exactly the record the table names
for it. Text that is not one of ManagementErrorCode.All, details for a code
without any, a missing or foreign details record and an object of no
declared type are refused, with fixed text that does not repeat what was
given. The public constructor that took a ready-made payload is gone, so
details reach the wire only as the record's own construction made them.

Reading stays lenient: the parser holds an error as the wire carried it
(ManagementError.FromWire, internal), because a client must still be able
to show a code a newer service added. ParseReply refuses nothing new.

The tests that need an error whose details do not belong to its code now
read one from reply text, as a client would.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): the envelope types refuse the serializer whatever its options 5ca655b1fc

The four envelope types refused to be read only when the serializer was
given one of the contract's own two options objects, because the refusing
converters were added to those options and not declared on the types. With
default or web options a request of another version, with a malformed
operation and an unknown member, was read with no envelope check (audit
W33, review A finding 3).

ManagementRequest, ManagementReply, ManagementError and ManagementPayload
now name a refusing converter in an attribute on the type, so the refusal
holds for every options object, the default ones included. It covers
writing too: options of a caller's choosing could indent an envelope across
lines, which the pipe's framing cannot carry. An envelope is therefore
written only by ManagementJson.Write, which writes it directly instead of
through the serializer, and read only by ParseRequest and ParseReply. The
text ManagementJson.Write produces is unchanged, byte for byte: the golden
files pass as they were.

Serializing an envelope type with ManagementJson.Options, which used to
work, is refused like any other options; ManagementJson.Write is the way.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): give the pipe contract its own rule for text on the wire 0f1003d122

Core's printable-text function was written to make pieces of sentences and
was used as the form of typed wire members. It sent the words "(missing)"
for a missing identity value and for an empty message, which a client could
not tell from a value; it added "..." after a cut, so a message could be
515 units and an identity value 203 where the record says 512 and 200; and
applied twice it changed the text (audit W34, review A finding 4). It was
also a public static on an authentication record with an internal constant
as its default limit (audit C14, review A finding 7).

The contract now has its own rule, ManagementText, in the Management
namespace:

- a code point that is not shown becomes a space, a lone surrogate U+FFFD;
- the cut keeps the bound exactly, never splits a surrogate pair and adds
  no mark, so a message is at most 512 units and an identity value 200;
- applied to its own result it changes nothing, so a client that reads a
  reply applies it again and holds the same text;
- a missing identity value stays absent: the two members of
  IdentityChangedDetails may be null and are then left out of the text;
- a message with nothing to show (missing, empty, blank once printable) is
  refused where the service builds an error. A client still reads a reply
  whose message is empty, so that it learns the code: ParseReply refuses
  nothing new.

IdentityChangedDetails applies the rule in its one constructor, which the
serializer uses too, so a client holds bounded values even from a service
that did not send them so.

Which characters are shown stays one definition, owned by Core, as
ADR-0004 says: it moves to a neutral type, Core's ServerText.IsShown, which
both rules call. QqqCompatibilityProblem.Printable stays for Core's
sentences, is internal again, and its comment no longer claims that every
space separator becomes a plain space.

No golden file changes: none held a cut or a missing value.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): carry an account's identity values one way in every reply 2b380a4ca0

The details of identity_changed made the principal and the tenant printable
and cut them at 200 units, while the snapshot, the result of servers.add
and the result of session.signIn carried the same values as the session
client holds them: up to 1,024 units, with only control characters
excluded, so a character that reorders text or a line separator would have
reached a client's screen through the snapshot (audit W35, review A
finding 5).

Those three results now say who is signed in through a record of the
contract, ServerAuthentication, in place of the service's own status
record. Its one constructor gives the principal and the tenant the form
IdentityChangedDetails gives them (ManagementText.Identity), the serializer
reads the wire through the same constructor, and the service builds it from
its status with ServerAuthentication.From. A test walks every result and
every details record and fails if an account is named by any other record.

The wire form does not change: the members, their names and their order
are those the status record had, so no golden file changes.

The comment on ManagementProtocol.MaximumIdentityCharacters no longer says
that the QQQ contract limits such a value to 200; it says what the session
client admits.

Not decided here, as the review left it for the handlers: the other text
in results that a server can influence (a server's status, the printer
sync detail, the last command and its outcome, job titles and messages).

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): enforce the server-name rule where a name enters, and prefix log lines with the short identifier cfeac4da50

The contract took any string as a server's name, and the service log's
per-server view put that name in front of every line. The activity log
file redacts each joined line a second time, so a server named `Cookie:`
lost its lines there; square brackets in a name forged structure inside a
line; and the view's own check of a name was a second, weaker copy of
Core's character rule (audit W39, review A finding 10).

The rule ADR-0004 decided ("Server names") is now a type of the contract,
ServerName: 1 to 64 UTF-16 units after trimming; every code point one that
Core's definition of printable text shows, with no surrogate without its
partner; no square brackets; and not shaped like a server identifier or
like the first eight or more characters of one, in either letter case.
ServersAddArgs and ServersRenameArgs cannot be built with a name that
breaks it, in code or from a request, and hold the name trimmed, so a
request that gives such a name is refused when its arguments are read,
which the dispatcher answers as invalid_request (ServerName.RefusalCode).
Both records now have a written-out constructor and no member that can be
set afterwards. Their wire form is unchanged, so no golden file changes.

ServiceLog.ForServer takes a server's full identifier and prefixes the
first eight characters of it, as the record says, instead of a name. The
code that made a name safe to prefix, and its 64-character limit, are gone
with the need for them; anything but a full identifier is refused. Nothing
calls the view yet.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

feat(management): servers.add carries the provider origin the check reported, and its refusal has a code 198663a54c

ADR-0004 ("Who names the sign-in provider") says servers.add carries the
sign-in provider's origin that servers.check reported, for comparison
only: the service reads the sign-in description once for the check and
again for the add, a server whose certificate is not verified could name
another provider the second time, and a stored certificate exception must
never cover an origin the operator did not see. The contract had no such
argument, its comment said the provider's address "is not an argument",
and the record left the refusal's error code unsettled (audit W56).

ServersAddArgs gains ProviderOrigin, optional and last on the wire, and
NamesTheProvider, the one rule for the comparison: the same text exactly,
or none on both sides. A request that leaves the origin out says the check
reported no provider, so leaving it out cannot skip the comparison.

The refusal gets a code of its own, provider_changed, the thirty-second in
the table, placed after the two codes of a failed check. No existing code
fits: invalid_request says the request was malformed, server_incompatible
and certificate_untrusted say a stage of the check failed and carry the
stages, server_refused says the QQQ server refused. The wizard has to tell
this refusal apart without reading the message, because it then runs the
check again and shows the provider the check now reports. The code carries
no details for the same reason, so it has no golden file of its own.

Golden file changed: servers.add.request.json gains the providerOrigin
member, because the example request now carries the provider the example
check reports.

ADR-0004 records the choice where it said the code was unsettled, gains
the table row, and no longer lists the missing argument as a gap.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): the operation table says that printers.discover is single-flight 356d4dd3a9

printers.discover is a read, and the table's comment on reads said they
never wait for another operation. But it runs a driver discovery in a
helper process, and nothing in the contract said whether concurrent
requests share one, so a hung driver and a client that retries could hold
several pipe instances, each with its own helper process (audit W36,
review A finding 6). ADR-0004 has since decided that it is single-flight.

ManagementOperationInfo gains SingleFlight, and the table sets it for
printers.discover and for nothing else: the computer runs one discovery at
a time, and a request that arrives while one is running, whoever started
it, joins it and gets its result; each request keeps its own time limit.
The comment on the Read kind now says what is true of reads: none waits
for an operation that changes something, and a single-flight read waits
for a run of its own operation.

This is the contract's side only. The discovery is made single-flight
where the pipe server is built; the table is what that code and its
clients read.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(logging): redact the ID-token hint, any authorization header, the code verifier and the session cookie's names f7080f6fee

The redaction let through values the product itself handles: the identity
provider's id_token_hint, an Authorization header of any scheme but
Bearer, the PKCE code_verifier, and the session cookie outside a cookie
header line (audit W40, review A finding 11). The review traced that no
log line carries any of them today; redaction is the second line of
defense, and ADR-0004 says the provider's sign-out request is never logged.

- id_token_hint, code_verifier, sessionUUID and sessionId are named
  secrets now: the value goes, the name and the rest of the line stay. The
  two cookie names are caught wherever they stand: bare, in JSON, on the
  continuation line of a folded header, under another header name.
- An Authorization or Proxy-Authorization header line loses its whole
  value to the end of the line, as a cookie header does, whatever the
  scheme or with none, and also as a member of a JSON object.

Two of the edges of audit W41 (review A finding 12) are closed because the
fix is small and can only hide the value of a callback's code or state:

- a quoted value (?code="..."&state='...') is redacted with its quotes;
- a percent-encoded callback inside another address (%3Fcode%3D...%26state%3D...)
  is redacted up to the encoded separator.

Left as they are, and recorded in tests as known behaviour: a cookie whose
name is not one of the product's two under a header name that only ends in
"Cookie", with other spacing, or folded; a bare code= or state= and their
JSON forms; a nonce and a client assertion; and every case of hiding too
much (a sentence with a colon after "cookie" or "authorization", an
ordinary ?state= or ?code= parameter). Narrowing a header pattern to fix
the last group would let a real header through, and no line the product
writes has those shapes.

A line that starts with "Authorization: Bearer" and goes on with other
credentials used to keep the text between them; it is now hidden to its
end. The test that holds old lines to their old redaction keeps the same
credentials without the header name.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): close the small gaps the retrospective review listed in the contract and the log e81ca8e9f6

Audit W37 (review A finding 8). The walk that refuses a null inside a list
the record knows handled lists and records only. It now also follows the
values of a dictionary, which are held to the rule a list's items are, and
a value type that may be null. No record of the contract has either; the
walk follows them so that one added later is not a way around the rule.

Audit W38 (review A finding 9).
- ManagementPayload.From refuses a value that is not written as a JSON
  object, as FromElement always did, instead of building a reply its
  client reports as incomplete.
- Six cases put a repeated member through ParseReply: in the envelope, in
  the result, in the error, in its details and under an escaped name. All
  were refused already; none was tested.
- The comments on ManagementReply and ServerRemoveResult now carry the two
  sentences the record gained: unsupported_version is answered in this
  version whatever was asked, and a servers.remove repeated after removal
  has finished gets server_not_found.
- Not changed: a reply with ok true, a result and an error is still read
  as a success with the error dropped (and the reverse). The review
  proposed no fix, and ParseReply gains no refusal the findings do not ask
  for.

Audit W42 (review A finding 13).
- The runtime hands its time provider to the service log, so the entries
  it keeps in memory are stamped by the injected clock like everything
  else the runtime times.
- The comment on ServiceLog no longer says the ring is what the companion
  shows: it is what the version 1 snapshot carries, the companion's Logs
  page reads the activity log file, and ADR-0004 removes the ring.
- The 200 ms real wait in the ring-order test stays, because it waits for
  something that must not happen; its summary now says so and that the
  audit counts it under W28.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs: record what the contract and redaction fixes closed, and settle the form of text on the wire 203c5f652e

Audit: W31 to W34, W37, W40, W42 and C14 are closed; W35, W36, W38, W39,
W41 and W56 say what is closed and what remains (Core's own bound on an
identity value and the other text a server can influence in results; the
discovery itself, which is built with the pipe server; the reply with both
a result and an error, for which the review proposed no fix; the code that
proposes a server's name; the redaction edges that are left; the
comparison servers.add makes, which is built with its handler).

ADR-0004: a new subsection, "Text on the wire", settles the question the
record had left open (a cut carries no mark, so 512 and 200 are exact) and
writes down the rest of the rule: applied twice it changes nothing, a
missing value is absent, a failure must say something, an account is
carried one way in every reply, and Core owns only which characters are
shown. The list of places where the contract did less than the record says
now states that the four are closed, and what the three later decisions
(server names, single-flight discovery, the provider's origin) still need
from the pipe server and the registry. The two code-table rows and the two
entries under "Resolved while reviewing the pipe contract" that describe
this are brought into line, with the edits listed where the record lists
its in-place edits.

ADR-0005 names provider_changed where it describes the refused add.
DEVELOPMENT.md describes the contract as it now is. CHANGELOG.md gains the
2.0 entry; the contract is not used by anything yet and the entry says so.

The Windows coverage minimum stays at 86: the gate measured 87.62% (3,909
of 4,461 lines), which rounds down to 87, less one.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

feat(management): one rule each for a server's name and its address, and a name proposed from an address cac9f9d1c2

The plan for the wizard stage puts the name rule and the address rule
once in the contract, for the service and the companion to call (its step
1, and its gap G3). This follows item 6 of this series, which added the
name rule as a check the arguments make.

ServerName gains what the companion needs. Check says which part of the
rule a name breaks (ServerNameProblem: empty, too long, not printable, a
square bracket, shaped like an identifier), reporting the first in that
order; the arguments of servers.add and servers.rename go on refusing
through it. Propose gives a name for a server's address that always
passes: the host by its ASCII name, without the brackets of an IPv6
literal and cut at 64 units; "Server" in front of a host shaped like an
identifier or its beginning; "Server" for an address with no host to name.
ADR-0004 had left how a name is proposed to the step that builds the
registry; the next commits record this.

ServerAddress.TryRead is the rule for the address an operator types:
HTTPS, or plain HTTP for this computer itself; no user information, query,
fragment or path; and a host with an ASCII name. It returns the origin in
the form QqqOrigin keeps one in, or says what is wrong
(ServerAddressProblem). It is the rule QqqSessionClient applies to the
origin it is built for, plus the last condition, and the tests hold it to
that: on a table of addresses the rule reads an address exactly when a
session client can be made for it, with the same origin, except for a host
with no ASCII name, which only the rule refuses.

Nothing is removed: the session client's own check and the sign-in flow's
copies of the address rule stay until their callers move to this one.
Nothing calls the two new functions yet.

The name tests move to ServerNameTests.cs, the file name the plan gives.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

feat(management): trust.set carries the provider origin too, and an origin is compared whenever it is given adf6f346f5

Two decisions taken with the plan for the wizard stage (its gaps G1 and
G2) change item 7 of this series.

trust.set has the race servers.add has: switching the exception on records
the provider the server names, from the service's own read, which the
operator has not seen. TrustSetArgs gains an optional ProviderOrigin, the
origin servers.check reported, compared with the service's read and
refused with the same code, provider_changed, with nothing changed.

The comparison has one rule for both operations, SignInProviderOrigin.Agrees:
an origin is compared whenever it is given, exactly; a request that
carries none is not compared. That replaces the rule item 7 gave
servers.add, under which a request without an origin said "the check
reported none" and was refused when the service read one.
ServersAddArgs.NamesTheProvider becomes ProviderAgrees. What the decided
rule does not catch is a server that named no provider at the check and
names one afterwards; the record notes it as a consequence.

provider_changed is described for both operations.

Golden file changed: trust.set.request.json gains the providerOrigin
member, because the example request now carries the provider the example
check reports. The code carries no details, so by the rule the golden
tests hold (one failure file per code that carries details) it has no
failure file of its own.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs: record the provider comparison for trust.set, the reasons for provider_changed, and the two rules in the contract 2596de59f5

ADR-0004, "Who names the sign-in provider": the reasons provider_changed
was chosen are the ones the decision gave (certificate_untrusted would say
Windows refused a certificate when none was; invalid_request could not be
told from a bad name; the wizard needs no details); the argument is absent
when the check named no provider and compared whenever it is given, with
the case that leaves uncaught written down as a consequence; and trust.set
is recorded beside it, with how an operator accepts a provider a server
names later, which "Certificate exception" had left unsaid. The code-table
row names both operations.

ADR-0004, "Server names": how a name is proposed is settled and is in the
contract; the rule is one function that says what is wrong; and the rule
for an address is beside it.

The audit rows W39 and W56, the changelog entry and DEVELOPMENT.md say the
same.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): an absent provider origin states that the check named none, and is compared ead4ea19f1

The provider origin a caller sends with servers.add and with trust.set
states what the check showed the operator. Under the rule of the commit
before last an origin was compared only when it was given, so a server
that named no provider at the check and one at the add was not caught:
exactly the provider the operator has not seen. The decision is reversed
(2026-11-10, under the owner's standing instruction): an absent origin
means "the check named no provider" and is compared like any other value.
If the service then reads a provider, the request is refused with
provider_changed. A caller that has not run a check cannot add a server
that uses a provider; it checks first, as the wizard and the command line
do. Nothing uses the contract yet, so no caller breaks.

SignInProviderOrigin.Agrees is still the one rule for both operations:
none shown and none read agree; none shown and one read, one shown and
none read, and one shown and another read are refused; the same one
agrees. Two origins are compared in the one form an origin is kept in
(QqqOrigin.Of: scheme, the host's ASCII name and port), not as text, so
one place written two ways is one place; text that names no origin agrees
with nothing. A theory holds each row for servers.add and for trust.set.

trust.set applies the rule when it switches the exception on. Switching it
off accepts nothing, so TrustSetArgs.ProviderAgrees is true for it
whatever the service reads.

ADR-0004 records the decision in place of the consequence it listed;
ADR-0005 says the wizard also returns to its first step when a provider
appears where the check named none; audit W56 no longer lists the
uncaught case; the changelog says what leaving the origin out means.

No golden file changes. provider_changed carries no details, and the
golden tests hold one failure file per code that carries details, so the
rule is left as it is and the code has no failure file.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(logging): redact the names the product's own bodies carry a secret under, in each shape eab3dcf37a

The named-secret pattern covered code_verifier, which the product never
writes, and not codeVerifier, the member of the code-exchange request
(review of the contract and redaction fixes, S1; audit W40). The session
client's and the API client's request and response bodies were read for
every member that carries a secret, and each is now a named value:

- codeVerifier, code_verifier, code-verifier and idTokenHint,
  id_token_hint, id-token-hint, in any letter case;
- uuid, the session identifier in a session response, at the start of a
  word only, so a device's or a job's identifier is left alone;
- enrollment_code beside enrollmentCode; deviceToken, sessionToken and the
  X-QQQ-Device-Token header were already covered by "token";
- code, in a message that also names a code verifier. That is the
  code-exchange request. Anywhere else "code" stays an ordinary word, so an
  exit code and an error object's code are still shown.

Every named value is also recognised with the equals sign percent-encoded
(name%3Dvalue), ending at an encoded separator, as the callback's code and
state already were.

A nonce is deliberately not redacted, under either of its names: it is not
a credential, and the test that records what is not recognised says why.

The documents say what is covered and what is not: audit W40 and the
changelog no longer say "wherever they stand" without the shapes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(logging): close the cheap redaction edges and record the rest d72f4cb9bb

The review of the redaction fix listed shapes that were neither covered
nor held by a test (nit 8; audit W41). The ones that are cheap and cannot
take a line an operator needs are closed:

- a cookie or authorization header whose own line holds nothing after the
  name: the value is then the next line, which is hidden, and the lines
  after it are kept;
- an authorization header written with an equals sign;
- HTTP_AUTHORIZATION and HTTP_COOKIE: the header's name may follow anything
  but a letter or a digit, which also makes the Proxy- group redundant, so
  it is removed (and Set- with it);
- a no-break space or a zero-width space between a name and its separator,
  for the two headers and for a named value;
- a callback whose ampersands are written as HTML writes them.

The rest is held by tests as known behaviour instead of being left
unstated: an unterminated quoted callback value; an arrow after a name; an
escaped quote inside a quoted value; the rest of a value after a comma, a
semicolon or a space; the second line of a header whose value begins on
its own line; and the lines these patterns hide although they hold no
secret, the new ones included (sessionId: 12, a line after one that ends
in "authorization:", authorization= in a sentence).

Audit W41 lists what is closed, what is not redacted and what is hidden.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): a surrogate without its partner is answered by the parser, never thrown 004784ab55

ParseRequest and ParseReply threw InvalidOperationException for the
escape of a surrogate without its partner ("\ud800" on its own, plain
ASCII on the wire) in the operation, the server selector, a member's name
or an error's code or message, and ArgumentException for text that holds
such a surrogate itself (review of the contract and redaction fixes, S2;
audit W74). Any administrator's program could send that to the pipe once
it is served. Older than the contract fixes.

The escape is not text, and it is answered where it stands, by the
distinction the parser already had for that place:

- in "op": a malformed operation; in "server": a malformed server;
- in a reply's code or message: an incomplete reply, as when either is not
  text. Not "not JSON", which the reviewer suggested for every case: a
  client takes that answer for a version 1 service, which a version 2
  reply with a bad message is not;
- in a member's name, at any depth, and for text that holds the surrogate
  itself: not JSON. The parse cannot read such a text far enough to know
  that no member is repeated, so nothing in it can be trusted.

In a value of the arguments, a result or the details the envelope is still
read and the record is still refused, as before.

ManagementJson.Write of a payload read with such an escape in a value
threw InvalidOperationException since the envelope is written by hand,
where the serializer threw JsonException. It throws JsonException again,
on purpose, and a test holds it.

The comments and ADR-0004 ("Text on the wire") said a surrogate without
its partner becomes U+FFFD and that a client holds printable text whatever
a service sent. That is true of a string; they now say what holds for the
escape on the wire.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): an IPv6 literal with a zone names no server address and no provider origin 2f08f3c124

SignInProviderOrigin.Agrees agreed for [fe80::1%25eth0] and
[fe80::1%25eth1], and for either and [fe80::1], and ServerAddress.TryRead
returned a zoned address without its zone (review of the contract and
redaction fixes, nit 1; audit C26). It was the one case found in which two
different places compared equal.

The origin form keeps the zone in the host it connects to and drops it
from its text, and the contract stores, compares and shows an origin as
text. So the contract's two rules refuse a zoned literal: the address rule
as a host that cannot be named, and the provider rule as text that names
no origin, which agrees with nothing.

Core is not changed. It compares the origins it holds with their zone
(the session client's same-origin rule, the certificate exception's
cover), so the fix belongs to the rule that compared text. What Core shows
for a zoned origin is recorded as open in the audit row.

The address rule's documentation also says that a caller builds the
session client from the origin the rule returns, not from the typed text
(nit 9): for a host written outside ASCII the two differ.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): a provider origin that names nothing is refused when the arguments are read eb0cdd9d33

"providerOrigin":"" or "none" was read by servers.add and trust.set and
could then never agree with what the service reads, so the request was
answered provider_changed however often the client checked again (review
of the contract and redaction fixes, nit 2; audit W56).

Text that names no origin states nothing the check could have shown. It is
a malformed argument: neither argument record can be built with it, in
code or from a request, so the request is refused as invalid_request when
its arguments are read, as a name that breaks the name rule is. For
trust.set that holds whichever way the switch goes.

"The check named no provider" is the member left out or given as null.
Both were read that way already; two tests now hold it, for both
operations.

The comparison table keeps the rows in which what the service read names
nothing, and the rows in which the request did are now rows of the refusal.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(management): the provider_changed code and the two operations state the current rule 6621534761

Three places still read like the rule before an absent origin was
compared: the comments on servers.add and trust.set in the operation
table, the summary of ProviderChanged, and the row for provider_changed in
the code table of ADR-0004 (review of the contract and redaction fixes,
nit 3). Each now says what is refused: the provider the service reads is
not the one the check reported, as the request states it, which includes
reading one where the check named none and none where it named one.

No behaviour changes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(management): say where the serializer's refusal of an envelope ends 8434163f46

The comments, audit W33, ADR-0004, the changelog and DEVELOPMENT.md said
the envelope types refuse the serializer "whatever its options". That is
slightly too strong (review of the contract and redaction fixes, nit 4):
options that carry a converter written for an envelope type are asked
before the declaration on the type, so with them the serializer writes
and reads that type as the converter says.

Each place now says what is true: the refusal holds for the default
options and for any options object that brings no converter of its own for
an envelope type. Such a converter has to be written on purpose, and
nothing in the product has one.

The test that tries the options is renamed to match, and a second test
holds where the refusal ends, with a converter of its own. The refusal's
message no longer says "whatever its options". No behaviour changes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): a request and a reply are built only by functions that check them 06375c74ac

Two ways around the contract's checks remained inside the assembly
(review of the contract and redaction fixes, nit 5; audit W75):

- ManagementReply and ManagementRequest were positional records. A reply
  could be built with any version, with an error beside a result or with
  neither, and a request with any text as its operation or its server.
- ManagementError.FromWire was internal and took a code, a message and
  details as given. The service will live in the same assembly.

A request and a reply now have no public constructor and no member that
can be set. They are built by ManagementRequest.For and by
ManagementReply.Success and Failure, and the parser builds them the same
way. So a request is always version 2 with an operation shaped like a
name and a server named by a full identifier or not at all, and a reply is
always version 2 with exactly one of a result and an error.

ManagementOperations.Check no longer asks about the version or the
spelling of the identifier: no request can have another, and the parser
is where unsupported_version is answered.

The parser's way into an error takes the error object as JSON and nothing
else, so no handler can use it to make an error from a code and a message
of its own.

Nothing on the wire changes; no golden file is touched. The tests that
built envelopes member by member use the functions, and the rows of the
routing check that no request can be built for are rows of the new test of
the request.

Left as it is and recorded in the audit row: the way in from JSON is
internal, and the parser's two result records can be built by hand.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): a server's name may not hold the fullwidth square brackets either 36bb3fc24e

The name rule refused "[" and "]" and accepted U+FF3B and U+FF3D (review
of the contract and redaction fixes, nit 10). Square brackets are refused
because they delimit the prefixes of the service log's lines, so a name
written into such a line could forge its structure. A program does not
take the fullwidth pair for a delimiter, but a person reading the line
does, so the pair is refused with the ASCII one. The rule's documentation
now gives that reason.

A name that begins with a hyphen, or is spelled like an option of the
command line, is still accepted, as ADR-0004's rule says. A test holds
that, and audit W76 records it as open for the step that builds the
command line's server commands, which must take such a name as a value or
change the rule.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): a server's name must hold something that draws 8b57fe28fb

Core's definition of a shown character goes by Unicode category, and by
category some characters are shown that draw nothing: the Hangul fillers
are letters, the blank Braille pattern is a symbol, and a combining mark
is shown with nothing to sit on. One of them could be a whole server name
(review of the contract and redaction fixes, nit 6), which is a server the
operator cannot see in a list or tell from another.

The name rule gains one condition of its own: at least one code point of
a name draws something, which is a letter, a digit, a punctuation mark or
a symbol other than the fillers (U+115F, U+1160, U+3164, U+FFA0) and the
blank Braille pattern (U+2800). A name made only of those, of combining
marks and of spaces is refused as ShowsNothing; the same characters stay
allowed beside one that draws.

Core's definition is not changed. The same characters can still be a
whole error message or identity value; audit C27 records that as open
against Core's definition.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(management): the message beside an account has the form of text on the wire f0d239a917

ServerAuthentication.Message was held as given, when the record was built
and when it was read: a message with a character that reorders text and a
line break came back raw through a result, and 2,005 units passed (review
of the contract and redaction fixes, nit 7). It is the service's own
wording today, but it is the one free-text member of the record whose
other two are bounded.

The record's one constructor now gives it the form of an error's message:
every character that is not shown becomes a space, and the text is cut at
512 UTF-16 units exactly. That holds for a record built from the service's
status and for one read from a reply. A message already in that form is
carried as it is, so no golden file changes.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(audit): name the change that closed a finding, so the status stays true after a merge 8bedf921b5

Fifteen statuses said "Closed by this change (2026-11-10)" or a variant
of it. After a squash merge nothing resolves "this change" (review of the
contract and redaction fixes, nit 11). Each now names the change by what
it is, the contract and redaction fixes that followed the retrospective
review, and the paragraph above the table says what that name covers: the
fixes and the fix round after their own review, merged as one pull
request.

No status changes its meaning.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(development): the contract paragraph follows the fix round 6020903f02

DEVELOPMENT.md describes the version 2 contract in one paragraph. It now
says what the fix round after the review of the contract and redaction
fixes changed: the parsing functions answer everything a text can hold and
never throw; a request and a reply are built only by functions that check
them; the message beside an account is bounded as an error's is; a
provider origin that names nothing is refused when the arguments are read;
and a caller builds the session client from the origin the address rule
returns, not from the typed text (nit 9).

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(management): a table holds the parsing functions to never throwing 89b56be748

The fix for the surrogate escape (audit W74) is held by the cases the
review found. This adds a table over the places a string or a name can
stand, in the envelope of a request and of a reply and beneath them, and
the fragments that are known to be trouble there: the escape of a high or
a low surrogate alone, doubled, in the wrong order or followed by another
character, the surrogate itself, an escape that is cut short, a NUL, a
line break, and a whole pair. Every text gets an answer from both
functions, and what was read goes through the routing check and the typed
readers without an exception.

It is a table of known shapes, not a fuzzer; audit R2 still stands.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(logging): look for the code verifier's name in the message as it was given 3779c51fa0

"code" is redacted only in a message that also names a code verifier. The
name was looked for in the text after the earlier rules had run, so a
message whose verifier name stood inside a value one of those rules hides
kept its code on another line (confirmation review of the contract and
redaction fixes, its one nit). The reviewer's probe:

  Cookie: a=b; codeVerifier=SYN-VER
  code=SYN-CODE

became "Cookie: [REDACTED]" and, on the next line, "code=SYN-CODE". The
same happened with the name on an authorization header's line, or inside
the quoted value of another named secret.

The name is now looked for in the message before any redaction (the
bounded text). The code's own pattern still runs on the redacted text, as
before, so nothing else about the rule changes.

Three rows in the test that holds every secret out of the ring, the sink
and the file cover the three shapes. With the old condition those three
rows fail and the other 235 tests of the class pass.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

james.maes merged commit 4e01519556 into release/2.0

2026-11-11 06:22:58 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qqq-print-agent#51