Skip to content

fix(wire): cap ParseRequest fields to max valid per command+type (PILOT-307)#6

Open
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-307-20260530-201729
Open

fix(wire): cap ParseRequest fields to max valid per command+type (PILOT-307)#6
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-307-20260530-201729

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What

ParseRequest uses strings.Fields which splits into an unbounded slice from any-length input. While the transport layer caps message size, this adds in-process protection by rejecting requests whose field count exceeds the maximum valid for the given command+record-type.

Why

Closes PILOT-307 (nameserver: strings.Fields is unbounded on input line).

How

Added maxRequestFieldsFor() that returns the expected maximum for each valid (command, type) pair, checked before the main switch block. Updated TestParseRequestExtraFields to expect rejection of over-field requests instead of silent acceptance.

Verification

  • go build ./...
  • go vet ./...
  • go test ./... -count=1
wire.go                    | 39 +++++++++++++++++++++++++++++++++++++++
 zz_fuzz_nameserver_test.go | 20 +++++++++++++-------
 2 files changed, 52 insertions(+), 7 deletions(-)

Closes https://vulturelabs.atlassian.net/browse/PILOT-307

…OT-307)

ParseRequest uses strings.Fields which splits into an unbounded slice
from any-length input. While the transport layer caps message size,
this adds in-process protection by rejecting requests whose field
count exceeds the maximum valid for the given command+record-type.

Added maxRequestFieldsFor() that returns the expected maximum for each
valid (command, type) pair, checked before the main switch block.
Existing tests updated — TestParseRequestExtraFields now expects
rejection of over-field requests instead of silent acceptance.

Closes PILOT-307
@matthew-pilot matthew-pilot added the matthew-fix-larger Medium scope PR by matthew-pilot (>50 LoC, ≤200 LoC) label May 30, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🔍 PR Explanation — #6 PILOT-307

What this does

Adds field-count validation to ParseRequest in wire.go, rejecting requests whose whitespace-delimited fields exceed the maximum valid count for the given command + record type.

The problem

ParseRequest uses strings.Fields which splits input into an unbounded slice. While the transport layer has a max-message-size cap, there was no in-process guard against pathological input that could trigger unbounded allocation.

The fix

1. New function maxRequestFieldsFor(cmd, rt string) int (wire.go:139–170)

  • Returns the expected max field count for each valid (command, type) pair:
    • QUERY A/N → 3, QUERY S → 4
    • REGISTER A/N → 4, REGISTER S → 6
  • Returns 0 for unknown commands (rejected later by the switch)

2. Pre-switch gate (wire.go:62–68)

  • Before the main command-type dispatch, checks len(fields) > maxFields
  • Returns a descriptive error: "too many fields (N > M)"

3. Test update (zz_fuzz_nameserver_test.go:147–160)

  • TestParseRequestExtraFields now expects rejection of over-field requests
  • Covers all 6 over-field scenarios (3 QUERY + 3 REGISTER) with table-driven test

Design choices

  • Separate maxRequestFieldsFor function keeps the validation table clean and standalone
  • The check runs after strings.ToUpper for cmd/rt, so field count is deterministic
  • Unknown commands get maxFields=0 and pass the check (they fail in the switch anyway) — avoids duplicate error paths

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

📊 PR Status — #6 PILOT-307

Field Value
State OPEN · MERGEABLE
Draft No
Branch openclaw/pilot-307-20260530-201729main
Files 2 files, +52/−7 (wire.go, zz_fuzz_nameserver_test.go)
Labels matthew-fix-larger
Author matthew-pilot
Created 2026-05-30T20:18:58Z

CI Checks (2/2 passing)

Check Result
test / ci ✅ pass
codecov/patch ✅ pass

Canary

Not yet triggered — canary label not present.

Linked Jira

PILOT-307 — nameserver: strings.Fields is unbounded on input line

Last operator activity

No operator (TeoSlayer) activity detected on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

matthew-fix-larger Medium scope PR by matthew-pilot (>50 LoC, ≤200 LoC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant