mirror of
https://github.com/docker/docker-agent.git
synced 2026-11-04 10:02:15 +00:00
fix(runtime): drop omitempty on CreateSessionRequest booleans
With `omitempty`, an explicit `false` is wire-indistinguishable from "unset". That's safe today (every server-side default for these flags is `false`) but a real forward-compat hazard the moment any default flips to `true": a client that explicitly opted out would suddenly be opted in. Always serialising the boolean keeps the sender's choice authoritative and removes the foot-gun before the wire schema solidifies. A new test pins the contract; the existing "empty struct" test is updated to reflect that booleans now always appear.
This commit is contained in:
2 files changed
+35
-4
No files matched your search
@@ -31,16 +31,22 @@ type LoadTeamRequest struct {
|
||||
// The same forward-compatibility argument as LoadTeamRequest applies: this
|
||||
// is the payload a remote backend will eventually send over the wire.
|
||||
//
|
||||
// Boolean fields intentionally do NOT use `omitempty`. With `omitempty`,
|
||||
// an explicit `false` is wire-indistinguishable from "unset", so a future
|
||||
// server-side default flip from `false` to `true` would silently change
|
||||
// the semantics for clients that omitted the field on purpose. Always
|
||||
// serialising the boolean keeps the sender's choice authoritative.
|
||||
//
|
||||
// GlobalPermissions is currently json:"-" (the permissions.Checker is an
|
||||
// opaque struct without a wire form yet); it'll get a serializable
|
||||
// representation in a follow-up.
|
||||
type CreateSessionRequest struct {
|
||||
AgentName string `json:"agent_name,omitempty"`
|
||||
ToolsApproved bool `json:"tools_approved,omitempty"`
|
||||
HideToolResults bool `json:"hide_tool_results,omitempty"`
|
||||
ToolsApproved bool `json:"tools_approved"`
|
||||
HideToolResults bool `json:"hide_tool_results"`
|
||||
SessionDB string `json:"session_db,omitempty"`
|
||||
ResumeSessionID string `json:"resume_session_id,omitempty"`
|
||||
SnapshotsEnabled bool `json:"snapshots_enabled,omitempty"`
|
||||
SnapshotsEnabled bool `json:"snapshots_enabled"`
|
||||
GlobalPermissions *permissions.Checker `json:"-"`
|
||||
WorkingDir string `json:"working_dir,omitempty"`
|
||||
}
|
||||
@@ -73,9 +73,34 @@ func TestCreateSessionRequest_RoundTrip(t *testing.T) {
|
||||
func TestCreateSessionRequest_OmitsEmptyFields(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Boolean fields are always serialized (no `omitempty`) so that an
|
||||
// explicit `false` survives the wire round-trip and stays distinct
|
||||
// from "unset" semantics owned by the server. String fields with
|
||||
// `omitempty` still drop out of the empty case.
|
||||
data, err := json.Marshal(CreateSessionRequest{})
|
||||
require.NoError(t, err)
|
||||
assert.JSONEq(t, `{}`, string(data))
|
||||
assert.JSONEq(t, `{"tools_approved":false,"hide_tool_results":false,"snapshots_enabled":false}`, string(data))
|
||||
}
|
||||
|
||||
func TestCreateSessionRequest_PreservesExplicitFalseOnTheWire(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Pins the contract that `omitempty` on booleans is intentionally
|
||||
// absent: a client sending an explicit `false` must reach the
|
||||
// server as `false`, not as the field's absence. If the server's
|
||||
// default ever flips to `true`, this guarantee keeps existing
|
||||
// clients' "no, really, false" intact.
|
||||
in := CreateSessionRequest{
|
||||
AgentName: "main",
|
||||
ToolsApproved: false,
|
||||
HideToolResults: false,
|
||||
SnapshotsEnabled: false,
|
||||
}
|
||||
data, err := json.Marshal(in)
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, string(data), `"tools_approved":false`)
|
||||
assert.Contains(t, string(data), `"hide_tool_results":false`)
|
||||
assert.Contains(t, string(data), `"snapshots_enabled":false`)
|
||||
}
|
||||
|
||||
func TestCreateSessionRequest_NonSerializableFieldsExcluded(t *testing.T) {
|
||||
|
||||
Reference in new issue
Block a user