Skip to content

Commit 5900b0f

Browse files
authored
refactor: make command.Command valid-by-construction (#21)
## Summary `command.Command` was `type Command string`, so its validity invariant — leading slash, lowercase, no trailing slash — was **not enforced by the type**. Three ways to hold an invalid command existed: 1. **Conversion** — `command.Command("FOO/")` is legal Go. 2. **Zero value** — `var c command.Command` is `""`, which `Parse` itself rejects. 3. **Unvalidated decode** — the generated wire path did a raw `command.Command(sval)` with no check. This PR models `Command` on the existing `did.DID` precedent: a struct with an unexported field, obtainable only through validating constructors. Invalid commands become unrepresentable. ## What changed - **`Command` is now `struct{ str string }`** (was `type Command string`). Added `MustParse`, `Undef`, and `Defined()`, mirroring `did.DID`. - **Serialization methods** (`MarshalCBOR`/`UnmarshalCBOR`/`MarshalDagJSON`/`UnmarshalDagJSON`/`MarshalJSON`/`UnmarshalJSON`) copied from `did.DID`. cbor-gen now **delegates** to them (the `(struct)` codegen path), and **decode validates** — a non-conforming command from the wire is rejected instead of silently accepted. - **Wire format unchanged.** A command still serializes as the same CBOR text string; fixtures and test vectors round-trip byte-identically (`delegation`/`invocation`/`container`/`receipt` tests stay green). - Regenerated the invocation/delegation datamodels. - `receipt.Command`: `const` → `var command.MustParse(...)` (can't `const` a struct). - `invocation.go`/`delegation.go`: `string(command)` → `command.String()` at the re-parse boundary. - Updated call sites to build commands via `MustParse`. The "bad command" test now feeds `command.Undef` — the only error path still reachable now that invalid commands can't be constructed. - Added `command_test.go` (the package previously had **no tests**): parse table, `MustParse`, CBOR/DAG-JSON round-trips, and the decode-validation guarantees. ## Notes - `New(segments...)` still trusts its caller's segments (like `cid`/`did` builders) rather than validating, to avoid a breaking signature change rippling into `bindcom`. The headline guarantee comes from the struct itself. - Independent of the in-flight `binding` work; this also makes a future `binding.Of(cmd command.Command)` a safe, error-free primitive.
1 parent 32d4a96 commit 5900b0f

16 files changed

Lines changed: 305 additions & 142 deletions

binding/handler_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/fil-forge/ucantone/ipld/datamodel"
1010
"github.com/fil-forge/ucantone/testutil"
1111
tdm "github.com/fil-forge/ucantone/testutil/datamodel"
12+
"github.com/fil-forge/ucantone/ucan/command"
1213
"github.com/fil-forge/ucantone/ucan/invocation"
1314
"github.com/stretchr/testify/require"
1415
)
@@ -24,7 +25,7 @@ func TestHandler(t *testing.T) {
2425
inv, err := invocation.Invoke(
2526
alice,
2627
alice.DID(),
27-
"/test/handler",
28+
command.MustParse("/test/handler"),
2829
datamodel.Map{"bytes": []byte{0x01, 0x02, 0x03}},
2930
)
3031
require.NoError(t, err)

examples/container_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/fil-forge/ucantone/ipld/datamodel"
88
"github.com/fil-forge/ucantone/principal/ed25519"
9+
"github.com/fil-forge/ucantone/ucan/command"
910
"github.com/fil-forge/ucantone/ucan/container"
1011
"github.com/fil-forge/ucantone/ucan/delegation"
1112
"github.com/fil-forge/ucantone/ucan/delegation/policy"
@@ -30,7 +31,7 @@ func TestContainer(t *testing.T) {
3031
mailer,
3132
alice.DID(),
3233
mailer.DID(),
33-
"/message/send",
34+
command.MustParse("/message/send"),
3435
delegation.WithPolicyBuilder(
3536
policy.All(".to", policy.Like(".", "*.example.com")),
3637
),
@@ -44,7 +45,7 @@ func TestContainer(t *testing.T) {
4445
inv, err := invocation.Invoke(
4546
alice,
4647
mailer.DID(),
47-
"/message/send",
48+
command.MustParse("/message/send"),
4849
datamodel.Map{
4950
"to": []string{"bob@example.com"},
5051
"subject": "Hello!",

examples/delegations_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
"github.com/fil-forge/ucantone/principal/ed25519"
7+
"github.com/fil-forge/ucantone/ucan/command"
78
"github.com/fil-forge/ucantone/ucan/delegation"
89
"github.com/fil-forge/ucantone/ucan/delegation/policy"
910
)
@@ -27,10 +28,10 @@ func TestDelegations(t *testing.T) {
2728

2829
// delegate alice capability to use the email service
2930
_, err = delegation.Delegate(
30-
mailer, // issuer
31-
alice.DID(), // audience (receiver)
32-
mailer.DID(), // subject
33-
"/message/send", // command
31+
mailer, // issuer
32+
alice.DID(), // audience (receiver)
33+
mailer.DID(), // subject
34+
command.MustParse("/message/send"), // command
3435
)
3536
if err != nil {
3637
panic(err)
@@ -40,7 +41,7 @@ func TestDelegations(t *testing.T) {
4041
alice,
4142
bob.DID(),
4243
mailer.DID(),
43-
"/message/send",
44+
command.MustParse("/message/send"),
4445
// alice delegates bob capability to use the email service, but only allows
4546
// bob to send to example.com email addresses
4647
delegation.WithPolicyBuilder(

examples/invocations_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/fil-forge/ucantone/ipld/datamodel"
88
"github.com/fil-forge/ucantone/principal/ed25519"
9+
"github.com/fil-forge/ucantone/ucan/command"
910
"github.com/fil-forge/ucantone/ucan/delegation"
1011
"github.com/fil-forge/ucantone/ucan/invocation"
1112
)
@@ -24,10 +25,10 @@ func TestInvocations(t *testing.T) {
2425

2526
// delegate alice capability to use the email service
2627
dlg, err := delegation.Delegate(
27-
mailer, // issuer
28-
alice.DID(), // audience (receiver)
29-
mailer.DID(), // subject
30-
"/message/send", // command
28+
mailer, // issuer
29+
alice.DID(), // audience (receiver)
30+
mailer.DID(), // subject
31+
command.MustParse("/message/send"), // command
3132
)
3233
if err != nil {
3334
panic(err)
@@ -36,7 +37,7 @@ func TestInvocations(t *testing.T) {
3637
inv, err := invocation.Invoke(
3738
alice,
3839
mailer.DID(),
39-
"/message/send",
40+
command.MustParse("/message/send"),
4041
datamodel.Map{
4142
"from": "alice@example.com",
4243
"to": "bob@example.com",

examples/promises_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/fil-forge/ucantone/examples/types"
88
"github.com/fil-forge/ucantone/ipld/datamodel"
99
"github.com/fil-forge/ucantone/principal/ed25519"
10+
"github.com/fil-forge/ucantone/ucan/command"
1011
"github.com/fil-forge/ucantone/ucan/delegation"
1112
"github.com/fil-forge/ucantone/ucan/invocation"
1213
"github.com/fil-forge/ucantone/ucan/promise"
@@ -33,13 +34,13 @@ func TestPromises(t *testing.T) {
3334
}
3435

3536
// A delegation from the mailer to alice allowing her to send emails
36-
msgSendDlg, err := delegation.Delegate(mailer, alice.DID(), mailer.DID(), "/msg/send")
37+
msgSendDlg, err := delegation.Delegate(mailer, alice.DID(), mailer.DID(), command.MustParse("/msg/send"))
3738
if err != nil {
3839
panic(err)
3940
}
4041

4142
// A delegation from the mailing list to alice allowing her to read the emails
42-
listEmailsDlg, err := delegation.Delegate(mailingList, alice.DID(), mailingList.DID(), "/emails/list")
43+
listEmailsDlg, err := delegation.Delegate(mailingList, alice.DID(), mailingList.DID(), command.MustParse("/emails/list"))
4344
if err != nil {
4445
panic(err)
4546
}
@@ -49,7 +50,7 @@ func TestPromises(t *testing.T) {
4950
readListInv, err := invocation.Invoke(
5051
alice,
5152
mailingList.DID(),
52-
"/emails/list",
53+
command.MustParse("/emails/list"),
5354
datamodel.Map{"limit": 100},
5455
invocation.WithAudience(mailer.DID()),
5556
invocation.WithProofs(listEmailsDlg.Link()),
@@ -64,7 +65,7 @@ func TestPromises(t *testing.T) {
6465
msgSendInv, err := invocation.Invoke(
6566
alice,
6667
mailer.DID(),
67-
"/msg/send",
68+
command.MustParse("/msg/send"),
6869
datamodel.Map{
6970
"from": "alice@example.com",
7071
"to": datamodel.Map{"await/ok": readListInv.Task().Link()},

examples/typed_args_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/fil-forge/ucantone/examples/types"
1212
"github.com/fil-forge/ucantone/ipld/datamodel"
1313
"github.com/fil-forge/ucantone/principal/ed25519"
14+
"github.com/fil-forge/ucantone/ucan/command"
1415
"github.com/fil-forge/ucantone/ucan/invocation"
1516
"github.com/fil-forge/ucantone/ucan/receipt"
1617
)
@@ -44,7 +45,7 @@ func TestExtractTypedArgsFromInvocation(t *testing.T) {
4445
inv, err := invocation.Invoke(
4546
alice,
4647
alice.DID(), // subject
47-
"/example/echo",
48+
command.MustParse("/example/echo"),
4849
&types.EchoArguments{Message: "Hello, UCAN!"},
4950
invocation.WithMetadata(map[string]any{
5051
"trace_id": "abc-123",

ucan/command/command.go

Lines changed: 132 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package command
22

33
import (
4+
"bytes"
45
"fmt"
6+
"io"
57
"strings"
8+
9+
jsg "github.com/alanshaw/dag-json-gen"
10+
cbg "github.com/whyrusleeping/cbor-gen"
611
)
712

813
const separator = "/"
@@ -13,11 +18,33 @@ const separator = "/"
1318
// A [Command] is composed of a leading slash which is optionally followed by
1419
// one or more slash-separated Segments of lowercase characters.
1520
//
21+
// The underlying field is unexported so a Command can only be obtained from a
22+
// validating constructor ([New], [Parse], [MustParse] or [Top]). This makes
23+
// invalid Commands unrepresentable: a value of this type is always either the
24+
// undefined zero value or a well-formed command.
25+
//
26+
// Note: this is a struct rather than `type Command string` for two reasons —
27+
// it prevents arbitrary strings being converted into a Command, and cbor-gen
28+
// only recognises MarshalCBOR/UnmarshalCBOR on non-string (struct) types.
29+
//
1630
// [Command]: https://github.com/ucan-wg/spec#command
17-
type Command string
31+
type Command struct {
32+
str string
33+
}
34+
35+
// Undef is the zero value of Command, representing an undefined command. Using
36+
// Command{} directly is also acceptable.
37+
var Undef = Command{}
1838

19-
// New creates a validated command from the provided list of segment strings. An
20-
// error is returned if an invalid Command would be formed
39+
// Defined reports whether the Command holds a value (i.e. is not the undefined
40+
// zero value).
41+
func (c Command) Defined() bool {
42+
return c.str != ""
43+
}
44+
45+
// New creates a command from the provided segments. Segments are assumed to be
46+
// well-formed; New does not validate them. To validate untrusted input, use
47+
// [Parse].
2148
func New(segments ...string) Command {
2249
return Top().Join(segments...)
2350
}
@@ -28,20 +55,30 @@ func New(segments ...string) Command {
2855
// [segment structure]: https://github.com/ucan-wg/spec#segment-structure
2956
func Parse(s string) (Command, error) {
3057
if !strings.HasPrefix(s, "/") {
31-
return "", ErrRequiresLeadingSlash
58+
return Undef, ErrRequiresLeadingSlash
3259
}
3360

3461
if len(s) > 1 && strings.HasSuffix(s, "/") {
35-
return "", ErrDisallowsTrailingSlash
62+
return Undef, ErrDisallowsTrailingSlash
3663
}
3764

3865
if s != strings.ToLower(s) {
39-
return "", ErrRequiresLowercase
66+
return Undef, ErrRequiresLowercase
4067
}
4168

42-
// The leading slash will result in the first element from strings. Split
69+
// The leading slash will result in the first element from strings.Split
4370
// being an empty string which is removed as strings.Join will ignore it.
44-
return Command(s), nil
71+
return Command{str: s}, nil
72+
}
73+
74+
// MustParse is like [Parse] but panics if s is not a valid Command. It is
75+
// intended for package-level command definitions from constant strings.
76+
func MustParse(s string) Command {
77+
cmd, err := Parse(s)
78+
if err != nil {
79+
panic(fmt.Sprintf("command: MustParse(%q): %v", s, err))
80+
}
81+
return cmd
4582
}
4683

4784
// Top is the most powerful capability.
@@ -52,7 +89,7 @@ func Parse(s string) (Command, error) {
5289
//
5390
// [Top]: https://github.com/ucan-wg/spec#-aka-top
5491
func Top() Command {
55-
return Command(separator)
92+
return Command{str: separator}
5693
}
5794

5895
// Join appends segments to the end of this command using the required
@@ -65,8 +102,8 @@ func (c Command) Join(segments ...string) Command {
65102
if size == 0 {
66103
return c
67104
}
68-
buf := make([]byte, 0, len(c)+size+len(segments))
69-
buf = append(buf, []byte(c)...)
105+
buf := make([]byte, 0, len(c.str)+size+len(segments))
106+
buf = append(buf, []byte(c.str)...)
70107
for _, s := range segments {
71108
if s != "" {
72109
if len(buf) > 1 {
@@ -75,16 +112,16 @@ func (c Command) Join(segments ...string) Command {
75112
buf = append(buf, []byte(s)...)
76113
}
77114
}
78-
return Command(buf)
115+
return Command{str: string(buf)}
79116
}
80117

81118
// Segments returns the ordered segments that comprise the Command as a slice
82119
// of strings.
83120
func (c Command) Segments() []string {
84-
if c == separator {
121+
if c.str == separator {
85122
return nil
86123
}
87-
return strings.Split(string(c), separator)[1:]
124+
return strings.Split(c.str, separator)[1:]
88125
}
89126

90127
// Proves returns true if the command is identical or a parent of the given
@@ -96,16 +133,93 @@ func (c Command) Segments() []string {
96133
// https://github.com/ucan-wg/spec/blob/main/README.md#segment-structure
97134
func (c Command) Proves(other Command) bool {
98135
// fast-path, equivalent to the code below (verified with fuzzing)
99-
if !strings.HasPrefix(string(other), string(c)) {
136+
if !strings.HasPrefix(other.str, c.str) {
100137
return false
101138
}
102-
return c == separator || len(c) == len(other) || other[len(c)] == separator[0]
139+
return c.str == separator || len(c.str) == len(other.str) || other.str[len(c.str)] == separator[0]
103140
}
104141

105142
// String returns the composed representation the command. This is also the
106143
// required wire representation (before IPLD encoding occurs).
107144
func (c Command) String() string {
108-
return string(c)
145+
return c.str
146+
}
147+
148+
func (c Command) MarshalJSON() ([]byte, error) {
149+
var buf bytes.Buffer
150+
if err := c.MarshalDagJSON(&buf); err != nil {
151+
return nil, err
152+
}
153+
return buf.Bytes(), nil
154+
}
155+
156+
func (c *Command) UnmarshalJSON(b []byte) error {
157+
return c.UnmarshalDagJSON(bytes.NewReader(b))
158+
}
159+
160+
func (c Command) MarshalCBOR(w io.Writer) error {
161+
if c.str == "" {
162+
_, err := w.Write(cbg.CborNull)
163+
return err
164+
}
165+
cw := cbg.NewCborWriter(w)
166+
if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(c.str))); err != nil {
167+
return err
168+
}
169+
_, err := cw.WriteString(c.str)
170+
return err
171+
}
172+
173+
func (c *Command) UnmarshalCBOR(r io.Reader) error {
174+
cr := cbg.NewCborReader(r)
175+
b, err := cr.ReadByte()
176+
if err != nil {
177+
return err
178+
}
179+
if b != cbg.CborNull[0] {
180+
if err := cr.UnreadByte(); err != nil {
181+
return err
182+
}
183+
str, err := cbg.ReadStringWithMax(cr, 2048)
184+
if err != nil {
185+
return err
186+
}
187+
parsed, err := Parse(str)
188+
if err != nil {
189+
return err
190+
}
191+
*c = parsed
192+
}
193+
return nil
194+
}
195+
196+
func (c Command) MarshalDagJSON(w io.Writer) error {
197+
jw := jsg.NewDagJsonWriter(w)
198+
if c.str == "" {
199+
return jw.WriteNull()
200+
}
201+
return jw.WriteString(c.str)
109202
}
110203

111-
var _ fmt.Stringer = (*Command)(nil)
204+
func (c *Command) UnmarshalDagJSON(r io.Reader) error {
205+
jr := jsg.NewDagJsonReader(r)
206+
str, err := jr.ReadStringOrNull(jsg.MaxLength)
207+
if err != nil {
208+
return err
209+
}
210+
if str == nil {
211+
return nil
212+
}
213+
parsed, err := Parse(*str)
214+
if err != nil {
215+
return err
216+
}
217+
*c = parsed
218+
return nil
219+
}
220+
221+
var (
222+
_ fmt.Stringer = (*Command)(nil)
223+
_ cbg.CBORMarshaler = (*Command)(nil)
224+
_ cbg.CBORUnmarshaler = (*Command)(nil)
225+
)

0 commit comments

Comments
 (0)