Skip to content

Generate a payload visitor for admin service messages - #306

Open
pseudomuto wants to merge 1 commit into
mainfrom
admin-payload-visitor
Open

pseudomuto wants to merge 1 commit into
mainfrom
admin-payload-visitor

Conversation

@pseudomuto

Copy link
Copy Markdown
Contributor

The payload visitor in api/proxy only knows public API types, so it never reaches the payloads inside admin messages and replication tasks. This adds one that does, as groundwork for sealing admin traffic (including replication streams) the way workflow and operator traffic already is. Nothing calls it yet.

I've added a target option to genvisitor. With -target payloads it generates a visitor that walks the server-internal types under every admin request and response, and routes each kind of payload to a helper in interceptor (see admin_payloads.go):

  • Public API messages that can hold a payload go to proxy.VisitPayloads, which already covers everything below them.
  • Payloads held directly by server types go to the caller's visitor one at a time and are written back in place, since proxy.VisitPayloads won't take a bare Payload.
  • History event blobs are decoded, visited, and re-encoded. A blob that can't be decoded, or whose invalid UTF-8 can't be repaired, is an error rather than being passed along or emptied.

Every bytes, DataBlob, or Any field reachable from an admin message has to be classified as event blob, opaque, or ignored, with the reason for ignoring it next to it. Anything unclassified, a table entry that no longer matches, or recursion that would hide something below it fails generation, so a server upgrade adding such a field breaks the build instead of quietly sending it unsealed. HSM nodes nest, so a recursive helper walks them and declares the fields it reads; any other field of that type that could hold data also fails generation. Recursion over types that hold nothing worth visiting (queue predicates, replication task equivalents) is cut short.

Note

There is still outstanding work/thought to be done.

HSM and CHASM state, serialized mutable state, and serialized tasks cannot be sealed whole because the server decodes them, so non-empty ones are reported to a caller-supplied Opaque hook. The encryptor will decide what to do with them; when sealing, the plan is to fail closed.

@pseudomuto
pseudomuto added this pull request to stack #307 October 6, 2026 18:14
@pseudomuto
pseudomuto requested a review from a team as a code owner October 6, 2026 18:14
@pseudomuto

Copy link
Copy Markdown
Contributor Author

I know +3700 looks scary. Half the diff is generated (payload_visitor_gen.go), and much of the rest is tests. ‎cmd/tools/genvisitor/payloads.go‎ and interceptor/admin_payloads.go are the main files that needs review. The rest is, of course, open for comments, but doesn't require careful review because:

  • The changes in cmd/tools/genvisitor did NOT change the previously generated repair_utf8_gen.go file
  • TestGeneratePayloadsMatchesCheckedIn regenerates and compares the result with the committed file, so a hand edit, or a dependency bump that changes the output, fails CI (no drift)
  • TestVisitAdminPayloadsReachesEveryPayload sets one payload at a time at every place the schema allows one under an admin request or response and requires the visitor to see it (ensures it's complete)

Base automatically changed from fix-genvisitor-activity to main October 7, 2026 11:00
@pseudomuto
pseudomuto force-pushed the admin-payload-visitor branch 2 times, most recently from 762031f to 04960b0 Compare October 9, 2026 20:04
@pseudomuto

Copy link
Copy Markdown
Contributor Author

Updated this a bit based on internal discussion.

We visit every payload in public API messages, server-internal messages, history event blobs, and AddTasksRequest task blobs and encrypt them as we do for Workflow/Operator services. However, there are a few exceptions worth noting:

        {
		// CHASM component state. The server decodes it, so it cannot be sealed
		// whole, and decoding it here needs the server's component registry.
		"temporal.server.api.persistence.v1.ChasmNode.data": true,
		// Documented as the future home of every task attribute.
		"temporal.server.api.replication.v1.ReplicationTask.data": true,
		// Serialized mutable state, handed back and forth between import calls.
		"temporal.server.api.adminservice.v1.ImportWorkflowExecutionRequest.token":  true,
		"temporal.server.api.adminservice.v1.ImportWorkflowExecutionResponse.token": true,
		// HSM node data in a mutation, the same thing StateMachineNode.data holds.
		"temporal.server.api.persistence.v1.WorkflowMutableStateMutation.StateMachineNodeMutation.data": true,
		// HSM and CHASM task data. CHASM types come from the server's component
		// registry, which the proxy does not have.
		"temporal.server.api.persistence.v1.StateMachineTaskInfo.data":          true,
		"temporal.server.api.persistence.v1.ChasmComponentAttributes.Task.data": true,
		"temporal.server.api.persistence.v1.ChasmTaskInfo.data":                 true,
		// A serialized history task. Its category, which says what it decodes to,
		// is on the GetDLQTasksRequest, and the response is visited on its own.
		"temporal.server.api.common.v1.HistoryTask.blob": true,
	},

When these fields are encountered, they are sent to a handler (which by default will return an error to fail closed).

The payload visitor in api/proxy only knows public API types, so it
never reaches the payloads inside admin messages and replication tasks.
This adds one that does, as groundwork for sealing admin traffic
(including replication streams) the way workflow and operator traffic
already is. Nothing calls it yet.

I've added a target option to genvisitor. With -target payloads it
generates a visitor that walks the server-internal types under every
admin request and response, and routes each kind of payload to a
helper in `interceptor` (see admin_payloads.go):

- Public API messages that can hold a payload go to
  proxy.VisitPayloads, which already covers everything below them.
- Payloads held directly by server types go to the caller's visitor
  one at a time and are written back in place, since
  proxy.VisitPayloads won't take a bare Payload.
- History event blobs are decoded, visited, and re-encoded. A blob
  that can't be decoded, or whose invalid UTF-8 can't be repaired, is
  an error rather than being passed along or emptied.
- History tasks in AddTasks are decoded by their category, visited,
  and re-encoded, so only the HSM or CHASM data inside them is opaque.
  A task of an unknown category, or one that can't be decoded, is
  reported whole as opaque.

Every bytes, DataBlob, or Any field reachable from an admin message has
to be classified as event blob, opaque, or ignored, with the reason for
ignoring it next to it. Anything unclassified, a table entry that no
longer matches, or recursion that would hide something below it fails
generation, so a server upgrade adding such a field breaks the build
instead of quietly sending it unsealed. HSM nodes nest, so they are
walked by a recursive helper that declares the fields it reads, and any
other field of that type that could hold data also fails generation.
Recursion over types that hold nothing worth visiting (queue
predicates, replication task equivalents) is cut short.

> [!NOTE]
> There is still outstanding work/thought to be done.

> HSM and CHASM state, serialized mutable state, and DLQ tasks (whose
> category is only on the request) cannot be sealed whole because the
> server decodes them, so non-empty ones are reported to a
> caller-supplied Opaque hook. The encryptor will decide what to do with
> them; when sealing, the plan is to fail closed.
@pseudomuto
pseudomuto force-pushed the admin-payload-visitor branch from 04960b0 to d6040f3 Compare October 9, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant