From 24293c113b0421f9b84c38ef983d57ae6d150fdb Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Mon, 28 Sep 2026 15:16:13 +0200 Subject: [PATCH 1/9] feat(s3): add the /s3/bucket/policy command Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests to Hilt as one command; the request's method selects the operation. PolicyArguments carries the forwarded request and, on a PUT, the policy document as the body. PolicyOK carries the policy's ETag and, on a GET, the canonical JSON document. Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/cbor_gen.go | 286 +++++++++++++++++++++++++++++++++ commands/s3/bucket/gen/main.go | 2 + commands/s3/bucket/json_gen.go | 275 +++++++++++++++++++++++++++++++ commands/s3/bucket/policy.go | 13 ++ commands/s3/bucket/types.go | 20 +++ 5 files changed, 596 insertions(+) create mode 100644 commands/s3/bucket/policy.go diff --git a/commands/s3/bucket/cbor_gen.go b/commands/s3/bucket/cbor_gen.go index cb7caff..3144c97 100644 --- a/commands/s3/bucket/cbor_gen.go +++ b/commands/s3/bucket/cbor_gen.go @@ -296,6 +296,292 @@ func (t *ListArguments) UnmarshalCBOR(r io.Reader) (err error) { return nil } +func (t *PolicyArguments) MarshalCBOR(w io.Writer) error { + if t == nil { + _, err := w.Write(cbg.CborNull) + return err + } + + cw := cbg.NewCborWriter(w) + + if _, err := cw.Write([]byte{162}); err != nil { + return err + } + + // t.Body ([]uint8) (slice) + if len("body") > 8192 { + return xerrors.Errorf("Value in field \"body\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("body"))); err != nil { + return err + } + if _, err := cw.WriteString(string("body")); err != nil { + return err + } + + if len(t.Body) > 2097152 { + return xerrors.Errorf("Byte array in field t.Body was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajByteString, uint64(len(t.Body))); err != nil { + return err + } + + if _, err := cw.Write(t.Body); err != nil { + return err + } + + // t.Request (s3.Request) (struct) + if len("request") > 8192 { + return xerrors.Errorf("Value in field \"request\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("request"))); err != nil { + return err + } + if _, err := cw.WriteString(string("request")); err != nil { + return err + } + + if err := t.Request.MarshalCBOR(cw); err != nil { + return err + } + return nil +} + +func (t *PolicyArguments) UnmarshalCBOR(r io.Reader) (err error) { + *t = PolicyArguments{} + + cr := cbg.NewCborReader(r) + + maj, extra, err := cr.ReadHeader() + if err != nil { + return err + } + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + + if maj != cbg.MajMap { + return fmt.Errorf("cbor input should be of type map") + } + + if extra > cbg.MaxLength { + return fmt.Errorf("PolicyArguments: map struct too large (%d)", extra) + } + + n := extra + + nameBuf := make([]byte, 7) + for i := uint64(0); i < n; i++ { + nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) + if err != nil { + return err + } + + if !ok { + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { + return err + } + continue + } + + switch string(nameBuf[:nameLen]) { + // t.Body ([]uint8) (slice) + case "body": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + + if extra > 2097152 { + return fmt.Errorf("t.Body: byte array too large (%d)", extra) + } + if maj != cbg.MajByteString { + return fmt.Errorf("expected byte array") + } + + if extra > 0 { + t.Body = make([]uint8, extra) + } + + if _, err := io.ReadFull(cr, t.Body); err != nil { + return err + } + + // t.Request (s3.Request) (struct) + case "request": + + { + + if err := t.Request.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling t.Request: %w", err) + } + + } + + default: + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { + return err + } + } + } + + return nil +} +func (t *PolicyOK) MarshalCBOR(w io.Writer) error { + if t == nil { + _, err := w.Write(cbg.CborNull) + return err + } + + cw := cbg.NewCborWriter(w) + + if _, err := cw.Write([]byte{162}); err != nil { + return err + } + + // t.ETag (string) (string) + if len("etag") > 8192 { + return xerrors.Errorf("Value in field \"etag\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("etag"))); err != nil { + return err + } + if _, err := cw.WriteString(string("etag")); err != nil { + return err + } + + if len(t.ETag) > 8192 { + return xerrors.Errorf("Value in field t.ETag was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.ETag))); err != nil { + return err + } + if _, err := cw.WriteString(string(t.ETag)); err != nil { + return err + } + + // t.Policy ([]uint8) (slice) + if len("policy") > 8192 { + return xerrors.Errorf("Value in field \"policy\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("policy"))); err != nil { + return err + } + if _, err := cw.WriteString(string("policy")); err != nil { + return err + } + + if len(t.Policy) > 2097152 { + return xerrors.Errorf("Byte array in field t.Policy was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajByteString, uint64(len(t.Policy))); err != nil { + return err + } + + if _, err := cw.Write(t.Policy); err != nil { + return err + } + + return nil +} + +func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { + *t = PolicyOK{} + + cr := cbg.NewCborReader(r) + + maj, extra, err := cr.ReadHeader() + if err != nil { + return err + } + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + + if maj != cbg.MajMap { + return fmt.Errorf("cbor input should be of type map") + } + + if extra > cbg.MaxLength { + return fmt.Errorf("PolicyOK: map struct too large (%d)", extra) + } + + n := extra + + nameBuf := make([]byte, 6) + for i := uint64(0); i < n; i++ { + nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) + if err != nil { + return err + } + + if !ok { + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { + return err + } + continue + } + + switch string(nameBuf[:nameLen]) { + // t.ETag (string) (string) + case "etag": + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.ETag = string(sval) + } + // t.Policy ([]uint8) (slice) + case "policy": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + + if extra > 2097152 { + return fmt.Errorf("t.Policy: byte array too large (%d)", extra) + } + if maj != cbg.MajByteString { + return fmt.Errorf("expected byte array") + } + + if extra > 0 { + t.Policy = make([]uint8, extra) + } + + if _, err := io.ReadFull(cr, t.Policy); err != nil { + return err + } + + default: + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { + return err + } + } + } + + return nil +} func (t *InfoArguments) MarshalCBOR(w io.Writer) error { if t == nil { _, err := w.Write(cbg.CborNull) diff --git a/commands/s3/bucket/gen/main.go b/commands/s3/bucket/gen/main.go index 2f464bc..8e8fde8 100644 --- a/commands/s3/bucket/gen/main.go +++ b/commands/s3/bucket/gen/main.go @@ -27,6 +27,8 @@ func main() { bucket.CreateArguments{}, bucket.DeleteArguments{}, bucket.ListArguments{}, + bucket.PolicyArguments{}, + bucket.PolicyOK{}, bucket.InfoArguments{}, bucket.InfoOK{}, bucket.ListOK{}, diff --git a/commands/s3/bucket/json_gen.go b/commands/s3/bucket/json_gen.go index 16d1db1..e2a1456 100644 --- a/commands/s3/bucket/json_gen.go +++ b/commands/s3/bucket/json_gen.go @@ -293,6 +293,281 @@ func (t *ListArguments) UnmarshalDagJSON(r io.Reader) (err error) { return nil } +func (t *PolicyArguments) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if t == nil { + err := jw.WriteNull() + return err + } + if err := jw.WriteObjectOpen(); err != nil { + return err + } + written := false + + // t.Body ([]uint8) (slice) + if len("body") > 8192 { + return fmt.Errorf("string in field \"body\" was too long") + } + if err := jw.WriteString(string("body")); err != nil { + return fmt.Errorf("writing string for field \"body\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Body) > 2097152 { + return fmt.Errorf("byte array in field t.Body was too long") + } + + if err := jw.WriteBytes(t.Body); err != nil { + return fmt.Errorf("writing bytes for field t.Body: %w", err) + } + + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.Request (s3.Request) (struct) + if len("request") > 8192 { + return fmt.Errorf("string in field \"request\" was too long") + } + if err := jw.WriteString(string("request")); err != nil { + return fmt.Errorf("writing string for field \"request\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if err := t.Request.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field t.Request: %w", err) + } + if err := jw.WriteObjectClose(); err != nil { + return err + } + return nil +} +func (t *PolicyArguments) UnmarshalDagJSON(r io.Reader) (err error) { + *t = PolicyArguments{} + + jr := jsg.NewDagJsonReader(r) + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for PolicyArguments: %w", err) + } + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for PolicyArguments: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for PolicyArguments: %w", err) + } + } else { + for i := uint64(0); i < 8192; i++ { + name, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field PolicyArguments: string too large") + } + return fmt.Errorf("reading string for field PolicyArguments: %w", err) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field PolicyArguments: %w", err) + } + switch name { + + // t.Body ([]uint8) (slice) + case "body": + + { + bval, err := jr.ReadBytes(2097152) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading bytes for field t.Body: byte array too large") + } + return fmt.Errorf("reading bytes for field t.Body: %w", err) + } + if len(bval) > 0 { + t.Body = []uint8(bval) + } + } + + // t.Request (s3.Request) (struct) + case "request": + + if err := t.Request.UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling t.Request: %w", err) + } + + default: + // Field doesn't exist on this type, so ignore it + if err := jr.DiscardType(); err != nil { + return fmt.Errorf("ignoring field %s for PolicyArguments: %w", name, err) + } + } + + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field PolicyArguments: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("map too large for PolicyArguments") + } + } + } + + return nil +} +func (t *PolicyOK) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if t == nil { + err := jw.WriteNull() + return err + } + if err := jw.WriteObjectOpen(); err != nil { + return err + } + written := false + + // t.ETag (string) (string) + if len("etag") > 8192 { + return fmt.Errorf("string in field \"etag\" was too long") + } + if err := jw.WriteString(string("etag")); err != nil { + return fmt.Errorf("writing string for field \"etag\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.ETag) > 8192 { + return fmt.Errorf("string in field t.ETag was too long") + } + if err := jw.WriteString(string(t.ETag)); err != nil { + return fmt.Errorf("writing string for field t.ETag: %w", err) + } + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.Policy ([]uint8) (slice) + if len("policy") > 8192 { + return fmt.Errorf("string in field \"policy\" was too long") + } + if err := jw.WriteString(string("policy")); err != nil { + return fmt.Errorf("writing string for field \"policy\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Policy) > 2097152 { + return fmt.Errorf("byte array in field t.Policy was too long") + } + + if err := jw.WriteBytes(t.Policy); err != nil { + return fmt.Errorf("writing bytes for field t.Policy: %w", err) + } + + if err := jw.WriteObjectClose(); err != nil { + return err + } + return nil +} +func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { + *t = PolicyOK{} + + jr := jsg.NewDagJsonReader(r) + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for PolicyOK: %w", err) + } + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for PolicyOK: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for PolicyOK: %w", err) + } + } else { + for i := uint64(0); i < 8192; i++ { + name, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field PolicyOK: string too large") + } + return fmt.Errorf("reading string for field PolicyOK: %w", err) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field PolicyOK: %w", err) + } + switch name { + + // t.ETag (string) (string) + case "etag": + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field t.ETag: string too long") + } + return fmt.Errorf("reading string for field t.ETag: %w", err) + } + t.ETag = string(sval) + } + + // t.Policy ([]uint8) (slice) + case "policy": + + { + bval, err := jr.ReadBytes(2097152) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading bytes for field t.Policy: byte array too large") + } + return fmt.Errorf("reading bytes for field t.Policy: %w", err) + } + if len(bval) > 0 { + t.Policy = []uint8(bval) + } + } + + default: + // Field doesn't exist on this type, so ignore it + if err := jr.DiscardType(); err != nil { + return fmt.Errorf("ignoring field %s for PolicyOK: %w", name, err) + } + } + + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field PolicyOK: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("map too large for PolicyOK") + } + } + } + + return nil +} func (t *InfoArguments) MarshalDagJSON(w io.Writer) error { jw := jsg.NewDagJsonWriter(w) if t == nil { diff --git a/commands/s3/bucket/policy.go b/commands/s3/bucket/policy.go new file mode 100644 index 0000000..df888bd --- /dev/null +++ b/commands/s3/bucket/policy.go @@ -0,0 +1,13 @@ +//go:build !codegen + +package bucket + +import ( + "github.com/fil-forge/ucantone/binding" + "github.com/fil-forge/ucantone/ucan/command" +) + +// Policy is the `/s3/bucket/policy` command. Ingot invokes it on Hilt with a +// forwarded S3 GetBucketPolicy, PutBucketPolicy or DeleteBucketPolicy request; +// the request's method selects the operation. +var Policy = binding.Bind[*PolicyArguments, *PolicyOK](command.MustParse("/s3/bucket/policy")) diff --git a/commands/s3/bucket/types.go b/commands/s3/bucket/types.go index 4faf6b6..51c88c8 100644 --- a/commands/s3/bucket/types.go +++ b/commands/s3/bucket/types.go @@ -23,6 +23,26 @@ type ListArguments struct { Request s3.Request `cborgen:"request" dagjsongen:"request"` } +// PolicyArguments are the arguments to `/s3/bucket/policy`. +type PolicyArguments struct { + // Request is the AWS S3 GetBucketPolicy, PutBucketPolicy or + // DeleteBucketPolicy request. + Request s3.Request `cborgen:"request" dagjsongen:"request"` + // Body is the policy document a PutBucketPolicy request carries. It is + // empty for the other two operations. + Body []byte `cborgen:"body" dagjsongen:"body"` +} + +// PolicyOK is the successful result of `/s3/bucket/policy`. +type PolicyOK struct { + // ETag is the tag of the bucket's policy after the operation. It is empty + // after a DeleteBucketPolicy. + ETag string `cborgen:"etag" dagjsongen:"etag"` + // Policy is the stored policy document in its canonical JSON form. It is + // set only for a GetBucketPolicy. + Policy []byte `cborgen:"policy" dagjsongen:"policy"` +} + // InfoArguments are the arguments to `/s3/bucket/info`. type InfoArguments struct { // Name is the global bucket name to look up. From 12ca55583ee5982c3b718a4f8f9e28790e64bdc5 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Tue, 13 Oct 2026 10:47:51 +0200 Subject: [PATCH 2/9] feat(s3): return the bucket policy as a typed document PolicyOK.Policy is now a *BucketPolicy encoded in CBOR with the rest of the result, instead of the policy's canonical JSON bytes. BucketPolicy, Statement and Principal move here from Hilt. Principal keeps its Go shape {All, IDs} on the wire, so the generated codecs cover it; its JSON methods give the AWS form ("*" or {"AWS": [user ARNs]}), which Ingot writes back to the S3 client. PolicyArguments.Body stays the raw document: Hilt validates what the client sent. Co-Authored-By: Claude Opus 5.5 --- commands/s3/bucket/cbor_gen.go | 699 ++++++++++++++++++++++++++- commands/s3/bucket/gen/main.go | 3 + commands/s3/bucket/json_gen.go | 762 ++++++++++++++++++++++++++++-- commands/s3/bucket/policy_json.go | 128 +++++ commands/s3/bucket/policy_test.go | 81 ++++ commands/s3/bucket/types.go | 39 +- 6 files changed, 1656 insertions(+), 56 deletions(-) create mode 100644 commands/s3/bucket/policy_json.go create mode 100644 commands/s3/bucket/policy_test.go diff --git a/commands/s3/bucket/cbor_gen.go b/commands/s3/bucket/cbor_gen.go index 3144c97..9dcae85 100644 --- a/commands/s3/bucket/cbor_gen.go +++ b/commands/s3/bucket/cbor_gen.go @@ -442,8 +442,13 @@ func (t *PolicyOK) MarshalCBOR(w io.Writer) error { } cw := cbg.NewCborWriter(w) + fieldCount := 2 - if _, err := cw.Write([]byte{162}); err != nil { + if t.Policy == nil { + fieldCount-- + } + + if _, err := cw.Write(cbg.CborEncodeMajorType(cbg.MajMap, uint64(fieldCount))); err != nil { return err } @@ -470,35 +475,175 @@ func (t *PolicyOK) MarshalCBOR(w io.Writer) error { return err } - // t.Policy ([]uint8) (slice) - if len("policy") > 8192 { - return xerrors.Errorf("Value in field \"policy\" was too long") + // t.Policy (bucket.BucketPolicy) (struct) + if t.Policy != nil { + + if len("policy") > 8192 { + return xerrors.Errorf("Value in field \"policy\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("policy"))); err != nil { + return err + } + if _, err := cw.WriteString(string("policy")); err != nil { + return err + } + + if err := t.Policy.MarshalCBOR(cw); err != nil { + return err + } } + return nil +} + +func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { + *t = PolicyOK{} + + cr := cbg.NewCborReader(r) - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("policy"))); err != nil { + maj, extra, err := cr.ReadHeader() + if err != nil { return err } - if _, err := cw.WriteString(string("policy")); err != nil { + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + + if maj != cbg.MajMap { + return fmt.Errorf("cbor input should be of type map") + } + + if extra > cbg.MaxLength { + return fmt.Errorf("PolicyOK: map struct too large (%d)", extra) + } + + n := extra + + nameBuf := make([]byte, 6) + for i := uint64(0); i < n; i++ { + nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) + if err != nil { + return err + } + + if !ok { + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { + return err + } + continue + } + + switch string(nameBuf[:nameLen]) { + // t.ETag (string) (string) + case "etag": + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.ETag = string(sval) + } + // t.Policy (bucket.BucketPolicy) (struct) + case "policy": + + { + + b, err := cr.ReadByte() + if err != nil { + return err + } + if b != cbg.CborNull[0] { + if err := cr.UnreadByte(); err != nil { + return err + } + t.Policy = new(BucketPolicy) + if err := t.Policy.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling t.Policy pointer: %w", err) + } + } + + } + + default: + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { + return err + } + } + } + + return nil +} +func (t *BucketPolicy) MarshalCBOR(w io.Writer) error { + if t == nil { + _, err := w.Write(cbg.CborNull) + return err + } + + cw := cbg.NewCborWriter(w) + + if _, err := cw.Write([]byte{162}); err != nil { return err } - if len(t.Policy) > 2097152 { - return xerrors.Errorf("Byte array in field t.Policy was too long") + // t.Version (string) (string) + if len("version") > 8192 { + return xerrors.Errorf("Value in field \"version\" was too long") } - if err := cw.WriteMajorTypeHeader(cbg.MajByteString, uint64(len(t.Policy))); err != nil { + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("version"))); err != nil { return err } + if _, err := cw.WriteString(string("version")); err != nil { + return err + } + + if len(t.Version) > 8192 { + return xerrors.Errorf("Value in field t.Version was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.Version))); err != nil { + return err + } + if _, err := cw.WriteString(string(t.Version)); err != nil { + return err + } + + // t.Statements ([]bucket.Statement) (slice) + if len("statement") > 8192 { + return xerrors.Errorf("Value in field \"statement\" was too long") + } - if _, err := cw.Write(t.Policy); err != nil { + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("statement"))); err != nil { + return err + } + if _, err := cw.WriteString(string("statement")); err != nil { return err } + if len(t.Statements) > 8192 { + return xerrors.Errorf("Slice value in field t.Statements was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.Statements))); err != nil { + return err + } + for _, v := range t.Statements { + if err := v.MarshalCBOR(cw); err != nil { + return err + } + + } return nil } -func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { - *t = PolicyOK{} +func (t *BucketPolicy) UnmarshalCBOR(r io.Reader) (err error) { + *t = BucketPolicy{} cr := cbg.NewCborReader(r) @@ -517,12 +662,12 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { } if extra > cbg.MaxLength { - return fmt.Errorf("PolicyOK: map struct too large (%d)", extra) + return fmt.Errorf("BucketPolicy: map struct too large (%d)", extra) } n := extra - nameBuf := make([]byte, 6) + nameBuf := make([]byte, 9) for i := uint64(0); i < n; i++ { nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) if err != nil { @@ -538,8 +683,8 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { } switch string(nameBuf[:nameLen]) { - // t.ETag (string) (string) - case "etag": + // t.Version (string) (string) + case "version": { sval, err := cbg.ReadStringWithMax(cr, 8192) @@ -547,30 +692,534 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { return err } - t.ETag = string(sval) + t.Version = string(sval) } - // t.Policy ([]uint8) (slice) - case "policy": + // t.Statements ([]bucket.Statement) (slice) + case "statement": maj, extra, err = cr.ReadHeader() if err != nil { return err } - if extra > 2097152 { - return fmt.Errorf("t.Policy: byte array too large (%d)", extra) + if extra > 8192 { + return fmt.Errorf("t.Statements: array too large (%d)", extra) } - if maj != cbg.MajByteString { - return fmt.Errorf("expected byte array") + + if maj != cbg.MajArray { + return fmt.Errorf("expected cbor array") + } + + if extra > 0 { + t.Statements = make([]Statement, extra) + } + + for i := 0; i < int(extra); i++ { + { + var maj byte + var extra uint64 + var err error + _ = maj + _ = extra + _ = err + + { + + if err := t.Statements[i].UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling t.Statements[i]: %w", err) + } + + } + + } + } + + default: + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { + return err + } + } + } + + return nil +} +func (t *Statement) MarshalCBOR(w io.Writer) error { + if t == nil { + _, err := w.Write(cbg.CborNull) + return err + } + + cw := cbg.NewCborWriter(w) + fieldCount := 5 + + if t.Sid == "" { + fieldCount-- + } + + if _, err := cw.Write(cbg.CborEncodeMajorType(cbg.MajMap, uint64(fieldCount))); err != nil { + return err + } + + // t.Sid (string) (string) + if t.Sid != "" { + + if len("sid") > 8192 { + return xerrors.Errorf("Value in field \"sid\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("sid"))); err != nil { + return err + } + if _, err := cw.WriteString(string("sid")); err != nil { + return err + } + + if len(t.Sid) > 8192 { + return xerrors.Errorf("Value in field t.Sid was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.Sid))); err != nil { + return err + } + if _, err := cw.WriteString(string(t.Sid)); err != nil { + return err + } + } + + // t.Actions ([]string) (slice) + if len("action") > 8192 { + return xerrors.Errorf("Value in field \"action\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("action"))); err != nil { + return err + } + if _, err := cw.WriteString(string("action")); err != nil { + return err + } + + if len(t.Actions) > 8192 { + return xerrors.Errorf("Slice value in field t.Actions was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.Actions))); err != nil { + return err + } + for _, v := range t.Actions { + if len(v) > 8192 { + return xerrors.Errorf("Value in field v was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(v))); err != nil { + return err + } + if _, err := cw.WriteString(string(v)); err != nil { + return err + } + + } + + // t.Effect (bucket.Effect) (string) + if len("effect") > 8192 { + return xerrors.Errorf("Value in field \"effect\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("effect"))); err != nil { + return err + } + if _, err := cw.WriteString(string("effect")); err != nil { + return err + } + + if len(t.Effect) > 8192 { + return xerrors.Errorf("Value in field t.Effect was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.Effect))); err != nil { + return err + } + if _, err := cw.WriteString(string(t.Effect)); err != nil { + return err + } + + // t.Resources ([]string) (slice) + if len("resource") > 8192 { + return xerrors.Errorf("Value in field \"resource\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("resource"))); err != nil { + return err + } + if _, err := cw.WriteString(string("resource")); err != nil { + return err + } + + if len(t.Resources) > 8192 { + return xerrors.Errorf("Slice value in field t.Resources was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.Resources))); err != nil { + return err + } + for _, v := range t.Resources { + if len(v) > 8192 { + return xerrors.Errorf("Value in field v was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(v))); err != nil { + return err + } + if _, err := cw.WriteString(string(v)); err != nil { + return err + } + + } + + // t.Principal (bucket.Principal) (struct) + if len("principal") > 8192 { + return xerrors.Errorf("Value in field \"principal\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("principal"))); err != nil { + return err + } + if _, err := cw.WriteString(string("principal")); err != nil { + return err + } + + if err := t.Principal.MarshalCBOR(cw); err != nil { + return err + } + return nil +} + +func (t *Statement) UnmarshalCBOR(r io.Reader) (err error) { + *t = Statement{} + + cr := cbg.NewCborReader(r) + + maj, extra, err := cr.ReadHeader() + if err != nil { + return err + } + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + + if maj != cbg.MajMap { + return fmt.Errorf("cbor input should be of type map") + } + + if extra > cbg.MaxLength { + return fmt.Errorf("Statement: map struct too large (%d)", extra) + } + + n := extra + + nameBuf := make([]byte, 9) + for i := uint64(0); i < n; i++ { + nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) + if err != nil { + return err + } + + if !ok { + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { + return err + } + continue + } + + switch string(nameBuf[:nameLen]) { + // t.Sid (string) (string) + case "sid": + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.Sid = string(sval) + } + // t.Actions ([]string) (slice) + case "action": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + + if extra > 8192 { + return fmt.Errorf("t.Actions: array too large (%d)", extra) + } + + if maj != cbg.MajArray { + return fmt.Errorf("expected cbor array") } if extra > 0 { - t.Policy = make([]uint8, extra) + t.Actions = make([]string, extra) + } + + for i := 0; i < int(extra); i++ { + { + var maj byte + var extra uint64 + var err error + _ = maj + _ = extra + _ = err + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.Actions[i] = string(sval) + } + + } + } + // t.Effect (bucket.Effect) (string) + case "effect": + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.Effect = Effect(sval) + } + // t.Resources ([]string) (slice) + case "resource": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + + if extra > 8192 { + return fmt.Errorf("t.Resources: array too large (%d)", extra) + } + + if maj != cbg.MajArray { + return fmt.Errorf("expected cbor array") + } + + if extra > 0 { + t.Resources = make([]string, extra) + } + + for i := 0; i < int(extra); i++ { + { + var maj byte + var extra uint64 + var err error + _ = maj + _ = extra + _ = err + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.Resources[i] = string(sval) + } + + } + } + // t.Principal (bucket.Principal) (struct) + case "principal": + + { + + if err := t.Principal.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling t.Principal: %w", err) + } + } - if _, err := io.ReadFull(cr, t.Policy); err != nil { + default: + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { return err } + } + } + + return nil +} +func (t *Principal) MarshalCBOR(w io.Writer) error { + if t == nil { + _, err := w.Write(cbg.CborNull) + return err + } + + cw := cbg.NewCborWriter(w) + + if _, err := cw.Write([]byte{162}); err != nil { + return err + } + + // t.All (bool) (bool) + if len("all") > 8192 { + return xerrors.Errorf("Value in field \"all\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("all"))); err != nil { + return err + } + if _, err := cw.WriteString(string("all")); err != nil { + return err + } + + if err := cbg.WriteBool(w, t.All); err != nil { + return err + } + + // t.IDs ([]string) (slice) + if len("ids") > 8192 { + return xerrors.Errorf("Value in field \"ids\" was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("ids"))); err != nil { + return err + } + if _, err := cw.WriteString(string("ids")); err != nil { + return err + } + + if len(t.IDs) > 8192 { + return xerrors.Errorf("Slice value in field t.IDs was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.IDs))); err != nil { + return err + } + for _, v := range t.IDs { + if len(v) > 8192 { + return xerrors.Errorf("Value in field v was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(v))); err != nil { + return err + } + if _, err := cw.WriteString(string(v)); err != nil { + return err + } + + } + return nil +} + +func (t *Principal) UnmarshalCBOR(r io.Reader) (err error) { + *t = Principal{} + + cr := cbg.NewCborReader(r) + + maj, extra, err := cr.ReadHeader() + if err != nil { + return err + } + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + + if maj != cbg.MajMap { + return fmt.Errorf("cbor input should be of type map") + } + + if extra > cbg.MaxLength { + return fmt.Errorf("Principal: map struct too large (%d)", extra) + } + + n := extra + + nameBuf := make([]byte, 3) + for i := uint64(0); i < n; i++ { + nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) + if err != nil { + return err + } + + if !ok { + // Field doesn't exist on this type, so ignore it + if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { + return err + } + continue + } + + switch string(nameBuf[:nameLen]) { + // t.All (bool) (bool) + case "all": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + if maj != cbg.MajOther { + return fmt.Errorf("booleans must be major type 7") + } + switch extra { + case 20: + t.All = false + case 21: + t.All = true + default: + return fmt.Errorf("booleans are either major type 7, value 20 or 21 (got %d)", extra) + } + // t.IDs ([]string) (slice) + case "ids": + + maj, extra, err = cr.ReadHeader() + if err != nil { + return err + } + + if extra > 8192 { + return fmt.Errorf("t.IDs: array too large (%d)", extra) + } + + if maj != cbg.MajArray { + return fmt.Errorf("expected cbor array") + } + + if extra > 0 { + t.IDs = make([]string, extra) + } + + for i := 0; i < int(extra); i++ { + { + var maj byte + var extra uint64 + var err error + _ = maj + _ = extra + _ = err + + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } + + t.IDs[i] = string(sval) + } + + } + } default: // Field doesn't exist on this type, so ignore it diff --git a/commands/s3/bucket/gen/main.go b/commands/s3/bucket/gen/main.go index 8e8fde8..c409c24 100644 --- a/commands/s3/bucket/gen/main.go +++ b/commands/s3/bucket/gen/main.go @@ -29,6 +29,9 @@ func main() { bucket.ListArguments{}, bucket.PolicyArguments{}, bucket.PolicyOK{}, + bucket.BucketPolicy{}, + bucket.Statement{}, + bucket.Principal{}, bucket.InfoArguments{}, bucket.InfoOK{}, bucket.ListOK{}, diff --git a/commands/s3/bucket/json_gen.go b/commands/s3/bucket/json_gen.go index e2a1456..32a90f5 100644 --- a/commands/s3/bucket/json_gen.go +++ b/commands/s3/bucket/json_gen.go @@ -454,30 +454,29 @@ func (t *PolicyOK) MarshalDagJSON(w io.Writer) error { return fmt.Errorf("writing string for field t.ETag: %w", err) } written = true - if written { - if err := jw.WriteComma(); err != nil { - return err + if t.Policy != nil { + if written { + if err := jw.WriteComma(); err != nil { + return err + } } } - // t.Policy ([]uint8) (slice) - if len("policy") > 8192 { - return fmt.Errorf("string in field \"policy\" was too long") - } - if err := jw.WriteString(string("policy")); err != nil { - return fmt.Errorf("writing string for field \"policy\": %w", err) - } - if err := jw.WriteObjectColon(); err != nil { - return err - } - if len(t.Policy) > 2097152 { - return fmt.Errorf("byte array in field t.Policy was too long") - } - - if err := jw.WriteBytes(t.Policy); err != nil { - return fmt.Errorf("writing bytes for field t.Policy: %w", err) + // t.Policy (bucket.BucketPolicy) (struct) + if t.Policy != nil { + if len("policy") > 8192 { + return fmt.Errorf("string in field \"policy\" was too long") + } + if err := jw.WriteString(string("policy")); err != nil { + return fmt.Errorf("writing string for field \"policy\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if err := t.Policy.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field t.Policy: %w", err) + } } - if err := jw.WriteObjectClose(); err != nil { return err } @@ -530,19 +529,23 @@ func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { t.ETag = string(sval) } - // t.Policy ([]uint8) (slice) + // t.Policy (bucket.BucketPolicy) (struct) case "policy": { - bval, err := jr.ReadBytes(2097152) + null, err := jr.PeekNull() if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading bytes for field t.Policy: byte array too large") - } - return fmt.Errorf("reading bytes for field t.Policy: %w", err) + return fmt.Errorf("peeking null for field t.Policy: %w", err) } - if len(bval) > 0 { - t.Policy = []uint8(bval) + if null { + if err := jr.ReadNull(); err != nil { + return fmt.Errorf("reading null for field t.Policy: %w", err) + } + } else { + t.Policy = new(BucketPolicy) + if err := t.Policy.UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling t.Policy pointer: %w", err) + } } } @@ -568,6 +571,709 @@ func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { return nil } +func (t *BucketPolicy) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if t == nil { + err := jw.WriteNull() + return err + } + if err := jw.WriteObjectOpen(); err != nil { + return err + } + written := false + + // t.Statements ([]bucket.Statement) (slice) + if len("statement") > 8192 { + return fmt.Errorf("string in field \"statement\" was too long") + } + if err := jw.WriteString(string("statement")); err != nil { + return fmt.Errorf("writing string for field \"statement\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Statements) > 8192 { + return fmt.Errorf("slice value in field t.Statements was too long") + } + + if err := jw.WriteArrayOpen(); err != nil { + return fmt.Errorf("writing array open for field t.Statements: %w", err) + } + for i, v := range t.Statements { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.Statements: %w", err) + } + } + if err := v.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field v: %w", err) + } + } + if err := jw.WriteArrayClose(); err != nil { + return fmt.Errorf("writing array close for field t.Statements: %w", err) + } + + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.Version (string) (string) + if len("version") > 8192 { + return fmt.Errorf("string in field \"version\" was too long") + } + if err := jw.WriteString(string("version")); err != nil { + return fmt.Errorf("writing string for field \"version\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Version) > 8192 { + return fmt.Errorf("string in field t.Version was too long") + } + if err := jw.WriteString(string(t.Version)); err != nil { + return fmt.Errorf("writing string for field t.Version: %w", err) + } + if err := jw.WriteObjectClose(); err != nil { + return err + } + return nil +} +func (t *BucketPolicy) UnmarshalDagJSON(r io.Reader) (err error) { + *t = BucketPolicy{} + + jr := jsg.NewDagJsonReader(r) + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for BucketPolicy: %w", err) + } + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for BucketPolicy: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for BucketPolicy: %w", err) + } + } else { + for i := uint64(0); i < 8192; i++ { + name, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field BucketPolicy: string too large") + } + return fmt.Errorf("reading string for field BucketPolicy: %w", err) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field BucketPolicy: %w", err) + } + switch name { + + // t.Statements ([]bucket.Statement) (slice) + case "statement": + { + + if err := jr.ReadArrayOpen(); err != nil { + return fmt.Errorf("reading array open for field t.Statements: %w", err) + } + + close, err := jr.PeekArrayClose() + if err != nil { + return fmt.Errorf("peeking array close for field t.Statements: %w", err) + } + if close { + if err := jr.ReadArrayClose(); err != nil { + return fmt.Errorf("reading array close for field t.Statements: %w", err) + } + + } else { + for i := 0; i < 8192; i++ { + item := make([]Statement, 1) + + if err := item[0].UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling item[0]: %w", err) + } + + t.Statements = append(t.Statements, item[0]) + + close, err := jr.ReadArrayCloseOrComma() + if err != nil { + return fmt.Errorf("reading array close or comma for field t.Statements: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("reading array for field t.Statements: slice too large") + } + } + } + + } + + // t.Version (string) (string) + case "version": + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field t.Version: string too long") + } + return fmt.Errorf("reading string for field t.Version: %w", err) + } + t.Version = string(sval) + } + default: + // Field doesn't exist on this type, so ignore it + if err := jr.DiscardType(); err != nil { + return fmt.Errorf("ignoring field %s for BucketPolicy: %w", name, err) + } + } + + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field BucketPolicy: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("map too large for BucketPolicy") + } + } + } + + return nil +} +func (t *Statement) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if t == nil { + err := jw.WriteNull() + return err + } + if err := jw.WriteObjectOpen(); err != nil { + return err + } + written := false + + // t.Actions ([]string) (slice) + if len("action") > 8192 { + return fmt.Errorf("string in field \"action\" was too long") + } + if err := jw.WriteString(string("action")); err != nil { + return fmt.Errorf("writing string for field \"action\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Actions) > 8192 { + return fmt.Errorf("slice value in field t.Actions was too long") + } + + if err := jw.WriteArrayOpen(); err != nil { + return fmt.Errorf("writing array open for field t.Actions: %w", err) + } + for i, v := range t.Actions { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.Actions: %w", err) + } + } + if len(v) > 8192 { + return fmt.Errorf("string in field v was too long") + } + if err := jw.WriteString(string(v)); err != nil { + return fmt.Errorf("writing string for field v: %w", err) + } + } + if err := jw.WriteArrayClose(); err != nil { + return fmt.Errorf("writing array close for field t.Actions: %w", err) + } + + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.Effect (bucket.Effect) (string) + if len("effect") > 8192 { + return fmt.Errorf("string in field \"effect\" was too long") + } + if err := jw.WriteString(string("effect")); err != nil { + return fmt.Errorf("writing string for field \"effect\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Effect) > 8192 { + return fmt.Errorf("string in field t.Effect was too long") + } + if err := jw.WriteString(string(t.Effect)); err != nil { + return fmt.Errorf("writing string for field t.Effect: %w", err) + } + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.Principal (bucket.Principal) (struct) + if len("principal") > 8192 { + return fmt.Errorf("string in field \"principal\" was too long") + } + if err := jw.WriteString(string("principal")); err != nil { + return fmt.Errorf("writing string for field \"principal\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if err := t.Principal.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field t.Principal: %w", err) + } + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.Resources ([]string) (slice) + if len("resource") > 8192 { + return fmt.Errorf("string in field \"resource\" was too long") + } + if err := jw.WriteString(string("resource")); err != nil { + return fmt.Errorf("writing string for field \"resource\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Resources) > 8192 { + return fmt.Errorf("slice value in field t.Resources was too long") + } + + if err := jw.WriteArrayOpen(); err != nil { + return fmt.Errorf("writing array open for field t.Resources: %w", err) + } + for i, v := range t.Resources { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.Resources: %w", err) + } + } + if len(v) > 8192 { + return fmt.Errorf("string in field v was too long") + } + if err := jw.WriteString(string(v)); err != nil { + return fmt.Errorf("writing string for field v: %w", err) + } + } + if err := jw.WriteArrayClose(); err != nil { + return fmt.Errorf("writing array close for field t.Resources: %w", err) + } + + written = true + if t.Sid != "" { + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + } + + // t.Sid (string) (string) + if t.Sid != "" { + if len("sid") > 8192 { + return fmt.Errorf("string in field \"sid\" was too long") + } + if err := jw.WriteString(string("sid")); err != nil { + return fmt.Errorf("writing string for field \"sid\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.Sid) > 8192 { + return fmt.Errorf("string in field t.Sid was too long") + } + if err := jw.WriteString(string(t.Sid)); err != nil { + return fmt.Errorf("writing string for field t.Sid: %w", err) + } + } + if err := jw.WriteObjectClose(); err != nil { + return err + } + return nil +} +func (t *Statement) UnmarshalDagJSON(r io.Reader) (err error) { + *t = Statement{} + + jr := jsg.NewDagJsonReader(r) + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for Statement: %w", err) + } + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for Statement: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for Statement: %w", err) + } + } else { + for i := uint64(0); i < 8192; i++ { + name, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field Statement: string too large") + } + return fmt.Errorf("reading string for field Statement: %w", err) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field Statement: %w", err) + } + switch name { + + // t.Actions ([]string) (slice) + case "action": + { + + if err := jr.ReadArrayOpen(); err != nil { + return fmt.Errorf("reading array open for field t.Actions: %w", err) + } + + close, err := jr.PeekArrayClose() + if err != nil { + return fmt.Errorf("peeking array close for field t.Actions: %w", err) + } + if close { + if err := jr.ReadArrayClose(); err != nil { + return fmt.Errorf("reading array close for field t.Actions: %w", err) + } + + } else { + for i := 0; i < 8192; i++ { + item := make([]string, 1) + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field item[0]: string too long") + } + return fmt.Errorf("reading string for field item[0]: %w", err) + } + item[0] = string(sval) + } + t.Actions = append(t.Actions, item[0]) + + close, err := jr.ReadArrayCloseOrComma() + if err != nil { + return fmt.Errorf("reading array close or comma for field t.Actions: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("reading array for field t.Actions: slice too large") + } + } + } + + } + + // t.Effect (bucket.Effect) (string) + case "effect": + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field t.Effect: string too long") + } + return fmt.Errorf("reading string for field t.Effect: %w", err) + } + t.Effect = Effect(sval) + } + + // t.Principal (bucket.Principal) (struct) + case "principal": + + if err := t.Principal.UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling t.Principal: %w", err) + } + + // t.Resources ([]string) (slice) + case "resource": + { + + if err := jr.ReadArrayOpen(); err != nil { + return fmt.Errorf("reading array open for field t.Resources: %w", err) + } + + close, err := jr.PeekArrayClose() + if err != nil { + return fmt.Errorf("peeking array close for field t.Resources: %w", err) + } + if close { + if err := jr.ReadArrayClose(); err != nil { + return fmt.Errorf("reading array close for field t.Resources: %w", err) + } + + } else { + for i := 0; i < 8192; i++ { + item := make([]string, 1) + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field item[0]: string too long") + } + return fmt.Errorf("reading string for field item[0]: %w", err) + } + item[0] = string(sval) + } + t.Resources = append(t.Resources, item[0]) + + close, err := jr.ReadArrayCloseOrComma() + if err != nil { + return fmt.Errorf("reading array close or comma for field t.Resources: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("reading array for field t.Resources: slice too large") + } + } + } + + } + + // t.Sid (string) (string) + case "sid": + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field t.Sid: string too long") + } + return fmt.Errorf("reading string for field t.Sid: %w", err) + } + t.Sid = string(sval) + } + default: + // Field doesn't exist on this type, so ignore it + if err := jr.DiscardType(); err != nil { + return fmt.Errorf("ignoring field %s for Statement: %w", name, err) + } + } + + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field Statement: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("map too large for Statement") + } + } + } + + return nil +} +func (t *Principal) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if t == nil { + err := jw.WriteNull() + return err + } + if err := jw.WriteObjectOpen(); err != nil { + return err + } + written := false + + // t.All (bool) (bool) + if len("all") > 8192 { + return fmt.Errorf("string in field \"all\" was too long") + } + if err := jw.WriteString(string("all")); err != nil { + return fmt.Errorf("writing string for field \"all\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if err := jw.WriteBool(t.All); err != nil { + return fmt.Errorf("writing bool for field t.All: %w", err) + } + written = true + if written { + if err := jw.WriteComma(); err != nil { + return err + } + } + + // t.IDs ([]string) (slice) + if len("ids") > 8192 { + return fmt.Errorf("string in field \"ids\" was too long") + } + if err := jw.WriteString(string("ids")); err != nil { + return fmt.Errorf("writing string for field \"ids\": %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return err + } + if len(t.IDs) > 8192 { + return fmt.Errorf("slice value in field t.IDs was too long") + } + + if err := jw.WriteArrayOpen(); err != nil { + return fmt.Errorf("writing array open for field t.IDs: %w", err) + } + for i, v := range t.IDs { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.IDs: %w", err) + } + } + if len(v) > 8192 { + return fmt.Errorf("string in field v was too long") + } + if err := jw.WriteString(string(v)); err != nil { + return fmt.Errorf("writing string for field v: %w", err) + } + } + if err := jw.WriteArrayClose(); err != nil { + return fmt.Errorf("writing array close for field t.IDs: %w", err) + } + + if err := jw.WriteObjectClose(); err != nil { + return err + } + return nil +} +func (t *Principal) UnmarshalDagJSON(r io.Reader) (err error) { + *t = Principal{} + + jr := jsg.NewDagJsonReader(r) + defer func() { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + }() + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for Principal: %w", err) + } + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for Principal: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for Principal: %w", err) + } + } else { + for i := uint64(0); i < 8192; i++ { + name, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field Principal: string too large") + } + return fmt.Errorf("reading string for field Principal: %w", err) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field Principal: %w", err) + } + switch name { + + // t.All (bool) (bool) + case "all": + bval, err := jr.ReadBool() + if err != nil { + return fmt.Errorf("reading bool for field t.All: %w", err) + } + t.All = bval + + // t.IDs ([]string) (slice) + case "ids": + { + + if err := jr.ReadArrayOpen(); err != nil { + return fmt.Errorf("reading array open for field t.IDs: %w", err) + } + + close, err := jr.PeekArrayClose() + if err != nil { + return fmt.Errorf("peeking array close for field t.IDs: %w", err) + } + if close { + if err := jr.ReadArrayClose(); err != nil { + return fmt.Errorf("reading array close for field t.IDs: %w", err) + } + + } else { + for i := 0; i < 8192; i++ { + item := make([]string, 1) + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field item[0]: string too long") + } + return fmt.Errorf("reading string for field item[0]: %w", err) + } + item[0] = string(sval) + } + t.IDs = append(t.IDs, item[0]) + + close, err := jr.ReadArrayCloseOrComma() + if err != nil { + return fmt.Errorf("reading array close or comma for field t.IDs: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("reading array for field t.IDs: slice too large") + } + } + } + + } + default: + // Field doesn't exist on this type, so ignore it + if err := jr.DiscardType(); err != nil { + return fmt.Errorf("ignoring field %s for Principal: %w", name, err) + } + } + + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field Principal: %w", err) + } + if close { + break + } + if i == 8192-1 { + return fmt.Errorf("map too large for Principal") + } + } + } + + return nil +} func (t *InfoArguments) MarshalDagJSON(w io.Writer) error { jw := jsg.NewDagJsonWriter(w) if t == nil { diff --git a/commands/s3/bucket/policy_json.go b/commands/s3/bucket/policy_json.go new file mode 100644 index 0000000..f3cd833 --- /dev/null +++ b/commands/s3/bucket/policy_json.go @@ -0,0 +1,128 @@ +package bucket + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "strings" +) + +// Wildcard is the principal "*", naming every principal of the tenant. +const Wildcard = "*" + +// UserARNPrefix is the prefix of a principal's user ARN; the principal id +// follows it. +const UserARNPrefix = "arn:aws:iam:::user/" + +// UserARN returns the user ARN of principal id. +func UserARN(id string) string { return UserARNPrefix + id } + +// Everyone returns the principal set naming every principal of the tenant. +func Everyone() Principal { return Principal{All: true} } + +// Only returns the principal set naming exactly the given ids. +func Only(ids ...string) Principal { return Principal{IDs: ids} } + +// MarshalJSON encodes All as "*" and anything else as {"AWS": [user ARNs]}, +// nil encoding as an empty list. +func (p Principal) MarshalJSON() ([]byte, error) { + if p.All { + return json.Marshal(Wildcard) + } + arns := make([]string, len(p.IDs)) + for i, id := range p.IDs { + arns[i] = UserARN(id) + } + return json.Marshal(struct { + AWS []string `json:"AWS"` + }{arns}) +} + +// UnmarshalJSON accepts "*", {"AWS": "*"}, {"AWS": ""} or +// {"AWS": ["", ...]}, keeping each id once, in the order it first +// appears. +func (p *Principal) UnmarshalJSON(data []byte) error { + const shape = `must be "*" or {"AWS": user ARNs}` + data = bytes.TrimSpace(data) + if len(data) > 0 && data[0] == '"' { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return fmt.Errorf("Principal: %w", err) + } + if s != Wildcard { + return fmt.Errorf("Principal %q: a string principal must be %q", s, Wildcard) + } + *p = Everyone() + return nil + } + var obj map[string]json.RawMessage + if err := json.Unmarshal(data, &obj); err != nil || obj == nil { + return fmt.Errorf("Principal: %s", shape) + } + for k := range obj { + if k != "AWS" { + return fmt.Errorf("Principal: unknown key %q, %s", k, shape) + } + } + aws, ok := obj["AWS"] + if !ok { + return fmt.Errorf("Principal: %s", shape) + } + var one string + if json.Unmarshal(aws, &one) == nil && one == Wildcard { + *p = Everyone() + return nil + } + var arns stringOrList + if err := json.Unmarshal(aws, &arns); err != nil { + return fmt.Errorf("Principal: %s", shape) + } + ids := make([]string, len(arns)) + for i, arn := range arns { + if arn == Wildcard { + return fmt.Errorf("Principal %q must be the bare string or {\"AWS\": %q}, not a list entry", Wildcard, Wildcard) + } + id, ok := strings.CutPrefix(arn, UserARNPrefix) + if !ok || id == "" { + return fmt.Errorf("Principal %q: must be a user ARN %q", arn, UserARN("")) + } + ids[i] = id + } + *p = Principal{IDs: dedupe(ids)} + return nil +} + +// stringOrList is a JSON field AWS allows as a string or a list of strings. +type stringOrList []string + +func (l *stringOrList) UnmarshalJSON(data []byte) error { + data = bytes.TrimSpace(data) + if len(data) > 0 && data[0] == '"' { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return err + } + *l = stringOrList{s} + return nil + } + var ss []string + if err := json.Unmarshal(data, &ss); err != nil { + return errors.New("must be a string or a list of strings") + } + *l = ss + return nil +} + +// dedupe keeps the first occurrence of each id. +func dedupe(ids []string) []string { + seen := make(map[string]bool, len(ids)) + out := ids[:0] + for _, id := range ids { + if !seen[id] { + seen[id] = true + out = append(out, id) + } + } + return out +} diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go new file mode 100644 index 0000000..a3c1a2d --- /dev/null +++ b/commands/s3/bucket/policy_test.go @@ -0,0 +1,81 @@ +package bucket_test + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/fil-forge/libforge/commands/s3/bucket" + "github.com/stretchr/testify/require" +) + +func TestPolicyOKRoundTrip(t *testing.T) { + in := &bucket.PolicyOK{ + ETag: `"bafy"`, + Policy: &bucket.BucketPolicy{ + Version: "2012-10-17", + Statements: []bucket.Statement{ + { + Sid: "readers", + Effect: bucket.Allow, + Principal: bucket.Only("alice", "bob"), + Actions: []string{"s3:GetObject"}, + Resources: []string{"arn:aws:s3:::b/*"}, + }, + { + Effect: bucket.Deny, + Principal: bucket.Everyone(), + Actions: []string{"s3:DeleteObject"}, + Resources: []string{"arn:aws:s3:::b/*"}, + }, + }, + }, + } + + var cb bytes.Buffer + require.NoError(t, in.MarshalCBOR(&cb)) + var out bucket.PolicyOK + require.NoError(t, out.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Equal(t, *in, out) + + var jb bytes.Buffer + require.NoError(t, in.MarshalDagJSON(&jb)) + var outJSON bucket.PolicyOK + require.NoError(t, outJSON.UnmarshalDagJSON(bytes.NewReader(jb.Bytes()))) + require.Equal(t, *in, outJSON) + + aws, err := json.Marshal(out.Policy) + require.NoError(t, err) + require.JSONEq(t, `{"Version":"2012-10-17","Statement":[`+ + `{"Sid":"readers","Effect":"Allow","Principal":{"AWS":["arn:aws:iam:::user/alice","arn:aws:iam:::user/bob"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::b/*"]},`+ + `{"Effect":"Deny","Principal":"*","Action":["s3:DeleteObject"],"Resource":["arn:aws:s3:::b/*"]}]}`, string(aws)) +} + +func TestPolicyOKWithoutPolicy(t *testing.T) { + in := &bucket.PolicyOK{ETag: `"bafy"`} + var cb bytes.Buffer + require.NoError(t, in.MarshalCBOR(&cb)) + var out bucket.PolicyOK + require.NoError(t, out.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Nil(t, out.Policy) +} + +func TestPrincipalJSON(t *testing.T) { + for _, tc := range []struct { + in string + want bucket.Principal + }{ + {`"*"`, bucket.Everyone()}, + {`{"AWS":"*"}`, bucket.Everyone()}, + {`{"AWS":"arn:aws:iam:::user/alice"}`, bucket.Only("alice")}, + {`{"AWS":["arn:aws:iam:::user/alice","arn:aws:iam:::user/alice"]}`, bucket.Only("alice")}, + } { + var p bucket.Principal + require.NoError(t, json.Unmarshal([]byte(tc.in), &p), tc.in) + require.Equal(t, tc.want, p, tc.in) + } + for _, bad := range []string{`"alice"`, `{"AWS":["*"]}`, `{"AWS":"alice"}`, `{"Service":"x"}`, `{}`, `null`} { + var p bucket.Principal + require.Error(t, json.Unmarshal([]byte(bad), &p), bad) + } +} diff --git a/commands/s3/bucket/types.go b/commands/s3/bucket/types.go index 51c88c8..a791baf 100644 --- a/commands/s3/bucket/types.go +++ b/commands/s3/bucket/types.go @@ -38,9 +38,42 @@ type PolicyOK struct { // ETag is the tag of the bucket's policy after the operation. It is empty // after a DeleteBucketPolicy. ETag string `cborgen:"etag" dagjsongen:"etag"` - // Policy is the stored policy document in its canonical JSON form. It is - // set only for a GetBucketPolicy. - Policy []byte `cborgen:"policy" dagjsongen:"policy"` + // Policy is the stored policy document. It is set only for a + // GetBucketPolicy. + Policy *BucketPolicy `cborgen:"policy,omitempty" dagjsongen:"policy,omitempty"` +} + +// BucketPolicy is a bucket policy document. Its JSON encoding is the AWS +// bucket policy document. +type BucketPolicy struct { + Version string `cborgen:"version" dagjsongen:"version" json:"Version"` + Statements []Statement `cborgen:"statement" dagjsongen:"statement" json:"Statement"` +} + +// Effect is whether a [Statement] grants or withholds its actions. +type Effect string + +const ( + Allow Effect = "Allow" + Deny Effect = "Deny" +) + +// Statement grants (Allow) or withholds (Deny) actions on the bucket's +// resources to principals. Sid is an optional label the caller chooses. +type Statement struct { + Sid string `cborgen:"sid,omitempty" dagjsongen:"sid,omitempty" json:"Sid,omitempty"` + Effect Effect `cborgen:"effect" dagjsongen:"effect" json:"Effect"` + Principal Principal `cborgen:"principal" dagjsongen:"principal" json:"Principal"` + Actions []string `cborgen:"action" dagjsongen:"action" json:"Action"` + Resources []string `cborgen:"resource" dagjsongen:"resource" json:"Resource"` +} + +// Principal is the set of principals a statement applies to: every principal +// of the tenant (All), or the listed ids. Its JSON encoding is the string "*" +// or {"AWS": [user ARNs]} (see policy_json.go). +type Principal struct { + All bool `cborgen:"all" dagjsongen:"all"` + IDs []string `cborgen:"ids" dagjsongen:"ids"` } // InfoArguments are the arguments to `/s3/bucket/info`. From 64602c9819de6a3be6dba9a8f08575f18a3cae7c Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Tue, 13 Oct 2026 11:42:05 +0200 Subject: [PATCH 3/9] feat(s3): parse and check bucket policy documents BucketPolicy.UnmarshalJSON is now the strict parser Hilt had: exact property names, no unknown fields, a null Sid refused, and WellFormed's structural checks. The policy action allowlist (PolicyAction, PolicyActions, PolicyWildcard), the bucket and object ARN helpers, MaxSetSize and ErrInvalidPolicy (InvalidBucketPolicy) move here with it, so Hilt and Ingot use these types directly. Ordering statements into canonical form stays in Hilt. Co-Authored-By: Claude Opus 5.5 --- commands/s3/bucket/policy_json.go | 278 ++++++++++++++++++++++++++++-- commands/s3/bucket/policy_test.go | 104 +++++++++++ 2 files changed, 368 insertions(+), 14 deletions(-) diff --git a/commands/s3/bucket/policy_json.go b/commands/s3/bucket/policy_json.go index f3cd833..59c01a7 100644 --- a/commands/s3/bucket/policy_json.go +++ b/commands/s3/bucket/policy_json.go @@ -3,21 +3,121 @@ package bucket import ( "bytes" "encoding/json" - "errors" "fmt" + "slices" "strings" + + "github.com/fil-forge/ucantone/errors" +) + +// Version2012 and Version2008 are the policy language versions a document +// may declare. Forge evaluates both the same way. +const ( + Version2012 = "2012-10-17" + Version2008 = "2008-10-17" ) -// Wildcard is the principal "*", naming every principal of the tenant. +// Wildcard is the principal "*", naming every principal of the tenant. It is +// carried as the bare JSON string "*" or as {"AWS": "*"}; inside a list it is +// rejected. const Wildcard = "*" +// PolicyWildcard is the action "s3:*", standing for every policy action. +const PolicyWildcard = "s3:*" + // UserARNPrefix is the prefix of a principal's user ARN; the principal id // follows it. const UserARNPrefix = "arn:aws:iam:::user/" -// UserARN returns the user ARN of principal id. +// s3ARNPrefix and objectsSuffix make up the policy bucket's ARNs, the only +// resources a document names. +const ( + s3ARNPrefix = "arn:aws:s3:::" + objectsSuffix = "/*" +) + +// MaxSetSize bounds the statements of a policy and the principals of a +// statement. A real policy is far smaller. +const MaxSetSize = 4096 + +// policyActions is the allowlist of permissions a bucket policy statement may +// carry, sorted. The rest of the S3 permission vocabulary stays out on +// purpose: a key holding s3:CreateBucket or s3:DeleteBucket acts outside the +// policy that granted it, a key holding a policy operation could rewrite the +// policy that granted it, and every principal holds s3:ListAllMyBuckets. +var policyActions = []string{ + "s3:AbortMultipartUpload", + "s3:DeleteObject", + "s3:DeleteObjectVersion", + "s3:GetObject", + "s3:GetObjectLegalHold", + "s3:GetObjectRetention", + "s3:GetObjectVersion", + "s3:ListBucket", + "s3:ListBucketMultipartUploads", + "s3:ListBucketVersions", + "s3:ListMultipartUploadParts", + "s3:PutObject", + "s3:PutObjectLegalHold", + "s3:PutObjectRetention", +} + +// bucketActions are the policy actions that act on the bucket rather than on +// its objects; every other policy action is an object action. The split is +// versitygw's, which evaluates the policy against these ARNs. +var bucketActions = []string{ + "s3:ListBucket", + "s3:ListBucketMultipartUploads", + "s3:ListBucketVersions", +} + +// PolicyAction reports whether a may appear in a bucket policy statement. The +// wildcard [PolicyWildcard] is not a policy action. +func PolicyAction(a string) bool { + _, ok := slices.BinarySearch(policyActions, a) + return ok +} + +// PolicyActions returns a copy of the allowlist: every permission a bucket +// policy may grant, sorted, so that expanding [PolicyWildcard] is +// deterministic. +func PolicyActions() []string { return slices.Clone(policyActions) } + +// objectAction reports whether policy action a acts on objects. +func objectAction(a string) bool { return !slices.Contains(bucketActions, a) } + +// InvalidPolicyErrorName is the name of [ErrInvalidPolicy]. +const InvalidPolicyErrorName = "InvalidBucketPolicy" + +// ErrInvalidPolicy is returned for a policy document that is not well formed. +// The returned error wraps it with the reason. +var ErrInvalidPolicy = errors.New(InvalidPolicyErrorName, "invalid bucket policy") + +// UserARN returns the ARN a document names principal id by. func UserARN(id string) string { return UserARNPrefix + id } +// BucketARN returns the ARN of bucket itself, which bucket actions need. +func BucketARN(bucket string) string { return s3ARNPrefix + bucket } + +// ObjectsARN returns the ARN of every object in bucket, which object actions +// need. +func ObjectsARN(bucket string) string { return s3ARNPrefix + bucket + objectsSuffix } + +// ParseS3ARN splits arn:aws:s3::: or arn:aws:s3:::/* into the +// bucket and whether it names the bucket's objects. Any other shape, a key +// prefix or a wildcard in the bucket name included, is not ok. +func ParseS3ARN(arn string) (bucket string, objects bool, ok bool) { + rest, ok := strings.CutPrefix(arn, s3ARNPrefix) + if !ok { + return "", false, false + } + bucket, objects = strings.CutSuffix(rest, objectsSuffix) + if bucket == "" || strings.ContainsAny(bucket, "/*?") { + return "", false, false + } + return bucket, objects, true +} + // Everyone returns the principal set naming every principal of the tenant. func Everyone() Principal { return Principal{All: true} } @@ -41,33 +141,33 @@ func (p Principal) MarshalJSON() ([]byte, error) { // UnmarshalJSON accepts "*", {"AWS": "*"}, {"AWS": ""} or // {"AWS": ["", ...]}, keeping each id once, in the order it first -// appears. +// appears. Anything else is an error wrapping [ErrInvalidPolicy]. func (p *Principal) UnmarshalJSON(data []byte) error { const shape = `must be "*" or {"AWS": user ARNs}` data = bytes.TrimSpace(data) if len(data) > 0 && data[0] == '"' { var s string if err := json.Unmarshal(data, &s); err != nil { - return fmt.Errorf("Principal: %w", err) + return fmt.Errorf("Principal: %v: %w", err, ErrInvalidPolicy) } if s != Wildcard { - return fmt.Errorf("Principal %q: a string principal must be %q", s, Wildcard) + return fmt.Errorf("Principal %q: a string principal must be %q: %w", s, Wildcard, ErrInvalidPolicy) } *p = Everyone() return nil } var obj map[string]json.RawMessage if err := json.Unmarshal(data, &obj); err != nil || obj == nil { - return fmt.Errorf("Principal: %s", shape) + return fmt.Errorf("Principal: %s: %w", shape, ErrInvalidPolicy) } for k := range obj { if k != "AWS" { - return fmt.Errorf("Principal: unknown key %q, %s", k, shape) + return fmt.Errorf("Principal: unknown key %q, %s: %w", k, shape, ErrInvalidPolicy) } } aws, ok := obj["AWS"] if !ok { - return fmt.Errorf("Principal: %s", shape) + return fmt.Errorf("Principal: %s: %w", shape, ErrInvalidPolicy) } var one string if json.Unmarshal(aws, &one) == nil && one == Wildcard { @@ -76,16 +176,16 @@ func (p *Principal) UnmarshalJSON(data []byte) error { } var arns stringOrList if err := json.Unmarshal(aws, &arns); err != nil { - return fmt.Errorf("Principal: %s", shape) + return fmt.Errorf("Principal: %s: %w", shape, ErrInvalidPolicy) } ids := make([]string, len(arns)) for i, arn := range arns { if arn == Wildcard { - return fmt.Errorf("Principal %q must be the bare string or {\"AWS\": %q}, not a list entry", Wildcard, Wildcard) + return fmt.Errorf("Principal %q must be the bare string or {\"AWS\": %q}, not a list entry: %w", Wildcard, Wildcard, ErrInvalidPolicy) } id, ok := strings.CutPrefix(arn, UserARNPrefix) if !ok || id == "" { - return fmt.Errorf("Principal %q: must be a user ARN %q", arn, UserARN("")) + return fmt.Errorf("Principal %q: must be a user ARN %q: %w", arn, UserARN(""), ErrInvalidPolicy) } ids[i] = id } @@ -101,14 +201,14 @@ func (l *stringOrList) UnmarshalJSON(data []byte) error { if len(data) > 0 && data[0] == '"' { var s string if err := json.Unmarshal(data, &s); err != nil { - return err + return fmt.Errorf("%v: %w", err, ErrInvalidPolicy) } *l = stringOrList{s} return nil } var ss []string if err := json.Unmarshal(data, &ss); err != nil { - return errors.New("must be a string or a list of strings") + return fmt.Errorf("must be a string or a list of strings: %w", ErrInvalidPolicy) } *l = ss return nil @@ -126,3 +226,153 @@ func dedupe(ids []string) []string { } return out } + +// UnmarshalJSON parses a policy document strictly and refuses one that is not +// well formed, so an invalid policy never comes into existence: a field the +// schema does not define, a property name that differs from the schema in +// case, a malformed principal, or a document [WellFormed] rejects is an error +// wrapping [ErrInvalidPolicy]. Lists keep the order the document gives them. +func (p *BucketPolicy) UnmarshalJSON(data []byte) error { + if err := strictKeys(data); err != nil { + return err + } + // The wire form takes Action and Resource as a string or a list. + var w struct { + Version string + Statement []struct { + Sid string + Effect Effect + Principal Principal + Action stringOrList + Resource stringOrList + } + } + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(&w); err != nil { + if errors.Is(err, ErrInvalidPolicy) { + return fmt.Errorf("decoding policy: %w", err) + } + return fmt.Errorf("decoding policy: %v: %w", err, ErrInvalidPolicy) + } + d := BucketPolicy{Version: w.Version, Statements: make([]Statement, len(w.Statement))} + for i, st := range w.Statement { + d.Statements[i] = Statement{Sid: st.Sid, Effect: st.Effect, Principal: st.Principal, Actions: st.Action, Resources: st.Resource} + } + if err := WellFormed(d); err != nil { + return err + } + *p = d + return nil +} + +// strictKeys rejects any property name that is not spelled exactly as the +// schema defines it, and a null Sid. encoding/json matches field names +// case-insensitively, so DisallowUnknownFields alone would accept "statement", +// "effect" or "ACTION". +// A document this cannot parse is left to the decoder, which reports it. +func strictKeys(data []byte) error { + var top map[string]json.RawMessage + if err := json.Unmarshal(data, &top); err != nil { + return nil + } + for k := range top { + if k != "Version" && k != "Statement" { + return fmt.Errorf("decoding policy: unknown field %q: %w", k, ErrInvalidPolicy) + } + } + var statements []map[string]json.RawMessage + if err := json.Unmarshal(top["Statement"], &statements); err != nil { + return nil + } + for _, st := range statements { + for k, v := range st { + switch k { + case "Sid": + // encoding/json decodes null into "" without error, which would + // read as an absent Sid. + if string(bytes.TrimSpace(v)) == "null" { + return fmt.Errorf("decoding policy: Sid must be a string: %w", ErrInvalidPolicy) + } + case "Effect", "Principal", "Action", "Resource": + default: + return fmt.Errorf("decoding policy: unknown field %q: %w", k, ErrInvalidPolicy) + } + } + } + return nil +} + +// WellFormed is the structural check every decoded document passes: a +// recognized Version, at least one statement and at most [MaxSetSize], each +// with a recognized effect, naming [Wildcard] or at least one principal and +// at most MaxSetSize (and never Wildcard inside the list), holding at least +// one action, each a [PolicyAction] or [PolicyWildcard], and at least one +// resource, each an ARN of the one bucket every resource in the document +// names, covering what its actions act on: an object action needs the objects +// ARN in its statement, a bucket action the bucket ARN, and "s3:*" both. It +// returns an error wrapping [ErrInvalidPolicy] otherwise. +func WellFormed(d BucketPolicy) error { + if d.Version != Version2012 && d.Version != Version2008 { + return fmt.Errorf("policy Version %q must be %q or %q: %w", d.Version, Version2012, Version2008, ErrInvalidPolicy) + } + if len(d.Statements) == 0 { + return fmt.Errorf("Statement must not be empty: %w", ErrInvalidPolicy) + } + if len(d.Statements) > MaxSetSize { + return fmt.Errorf("Statement holds %d statements, at most %d are allowed: %w", len(d.Statements), MaxSetSize, ErrInvalidPolicy) + } + bucket := "" + for i, st := range d.Statements { + if st.Effect != Allow && st.Effect != Deny { + return fmt.Errorf("statement %d: Effect %q must be %q or %q: %w", i, st.Effect, Allow, Deny, ErrInvalidPolicy) + } + if !st.Principal.All { + if len(st.Principal.IDs) == 0 { + return fmt.Errorf("statement %d: Principal must not be empty: %w", i, ErrInvalidPolicy) + } + if len(st.Principal.IDs) > MaxSetSize { + return fmt.Errorf("statement %d: Principal names %d principals, at most %d are allowed: %w", i, len(st.Principal.IDs), MaxSetSize, ErrInvalidPolicy) + } + if slices.Contains(st.Principal.IDs, Wildcard) { + return fmt.Errorf("statement %d: Principal %q must be the bare string, not a list entry: %w", i, Wildcard, ErrInvalidPolicy) + } + } + if len(st.Actions) == 0 { + return fmt.Errorf("statement %d: Action must not be empty: %w", i, ErrInvalidPolicy) + } + for _, a := range st.Actions { + if a != PolicyWildcard && !PolicyAction(a) { + return fmt.Errorf("statement %d: Action %q is not a policy action: %w", i, a, ErrInvalidPolicy) + } + } + if len(st.Resources) == 0 { + return fmt.Errorf("statement %d: Resource must not be empty: %w", i, ErrInvalidPolicy) + } + var hasBucket, hasObjects bool + for _, r := range st.Resources { + b, objects, ok := ParseS3ARN(r) + if !ok { + return fmt.Errorf("statement %d: Resource %q must be %q or %q: %w", i, r, BucketARN(""), ObjectsARN(""), ErrInvalidPolicy) + } + if bucket == "" { + bucket = b + } + if b != bucket { + return fmt.Errorf("statement %d: Resource %q names bucket %q, the policy names %q: %w", i, r, b, bucket, ErrInvalidPolicy) + } + hasBucket = hasBucket || !objects + hasObjects = hasObjects || objects + } + for _, a := range st.Actions { + wildcard := a == PolicyWildcard + if (wildcard || objectAction(a)) && !hasObjects { + return fmt.Errorf("statement %d: Action %q needs Resource %q: %w", i, a, ObjectsARN(bucket), ErrInvalidPolicy) + } + if (wildcard || !objectAction(a)) && !hasBucket { + return fmt.Errorf("statement %d: Action %q needs Resource %q: %w", i, a, BucketARN(bucket), ErrInvalidPolicy) + } + } + } + return nil +} diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index a3c1a2d..ef49f6f 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -3,6 +3,7 @@ package bucket_test import ( "bytes" "encoding/json" + "strings" "testing" "github.com/fil-forge/libforge/commands/s3/bucket" @@ -79,3 +80,106 @@ func TestPrincipalJSON(t *testing.T) { require.Error(t, json.Unmarshal([]byte(bad), &p), bad) } } + +// policyJSON wraps statements, given as JSON, in a 2012-10-17 document. +func policyJSON(statements ...string) string { + return `{"Version": "2012-10-17", "Statement": [` + strings.Join(statements, ",") + `]}` +} + +// res is the Resource member covering every policy action on photos. +const res = `"Resource": ["arn:aws:s3:::photos", "arn:aws:s3:::photos/*"]` + +func TestBucketPolicyUnmarshalJSON(t *testing.T) { + t.Run("decodes the documented shape, keeping its order", func(t *testing.T) { + var d bucket.BucketPolicy + require.NoError(t, json.Unmarshal([]byte(policyJSON( + `{"Sid": "owners", "Effect": "Allow", "Principal": {"AWS": ["arn:aws:iam:::user/bob", "arn:aws:iam:::user/alice"]}, "Action": ["s3:ListBucket", "s3:GetObject"], `+res+`}`, + `{"Effect": "Deny", "Principal": "*", "Action": "s3:PutObjectRetention", "Resource": "arn:aws:s3:::photos/*"}`, + )), &d)) + require.Equal(t, bucket.BucketPolicy{Version: bucket.Version2012, Statements: []bucket.Statement{ + {Sid: "owners", Effect: bucket.Allow, Principal: bucket.Only("bob", "alice"), Actions: []string{"s3:ListBucket", "s3:GetObject"}, Resources: []string{bucket.BucketARN("photos"), bucket.ObjectsARN("photos")}}, + {Effect: bucket.Deny, Principal: bucket.Everyone(), Actions: []string{"s3:PutObjectRetention"}, Resources: []string{bucket.ObjectsARN("photos")}}, + }}, d) + }) + + rejects := []struct { + name string + body string + want string + }{ + {"a missing version", `{"Statement": [{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `}]}`, `Version "" must be`}, + {"an unknown version", `{"Version": "2024-01-01", "Statement": [{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `}]}`, `Version "2024-01-01" must be`}, + {"unknown top-level field", `{"Version": "2012-10-17", "Statement": [], "Id": "x"}`, `unknown field "Id"`}, + {"a condition", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `, "Condition": {}}`), `unknown field "Condition"`}, + {"a not-principal", policyJSON(`{"Effect": "Deny", "NotPrincipal": "*", "Action": ["s3:GetObject"], ` + res + `}`), `unknown field "NotPrincipal"`}, + {"old field names", `{"statements": [{"effect": "allow", "principals": ["alice"], "actions": ["s3:GetObject"]}]}`, `unknown field "statements"`}, + {"a string principal other than the wildcard", policyJSON(`{"Effect": "Allow", "Principal": "alice", "Action": ["s3:GetObject"], ` + res + `}`), `a string principal must be "*"`}, + {"a bare id list principal", policyJSON(`{"Effect": "Allow", "Principal": ["alice"], "Action": ["s3:GetObject"], ` + res + `}`), `must be "*" or {"AWS": user ARNs}`}, + {"an ARN with an account", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": "arn:aws:iam::123456789012:user/alice"}, "Action": ["s3:GetObject"], ` + res + `}`), `must be a user ARN`}, + {"a bare id inside AWS", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": ["alice"]}, "Action": ["s3:GetObject"], ` + res + `}`), `must be a user ARN`}, + {"a user ARN without an id", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": "arn:aws:iam:::user/"}, "Action": ["s3:GetObject"], ` + res + `}`), `must be a user ARN`}, + {"a role ARN", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": "arn:aws:iam:::role/alice"}, "Action": ["s3:GetObject"], ` + res + `}`), `must be a user ARN`}, + {"a principal key other than AWS", policyJSON(`{"Effect": "Allow", "Principal": {"Service": "s3.amazonaws.com"}, "Action": ["s3:GetObject"], ` + res + `}`), `unknown key "Service"`}, + {"a lowercase AWS key", policyJSON(`{"Effect": "Allow", "Principal": {"aws": "*"}, "Action": ["s3:GetObject"], ` + res + `}`), `unknown key "aws"`}, + {"AWS beside another key", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": "*", "Federated": "x"}, "Action": ["s3:GetObject"], ` + res + `}`), `unknown key "Federated"`}, + {"an empty principal object", policyJSON(`{"Effect": "Allow", "Principal": {}, "Action": ["s3:GetObject"], ` + res + `}`), `must be "*" or {"AWS": user ARNs}`}, + {"the wildcard inside the AWS list", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": ["arn:aws:iam:::user/alice", "*"]}, "Action": ["s3:GetObject"], ` + res + `}`), `not a list entry`}, + {"a lowercase top-level field", `{"Version": "2012-10-17", "statement": []}`, `unknown field "statement"`}, + {"a lowercase version field", `{"version": "2012-10-17", "Statement": []}`, `unknown field "version"`}, + {"a lowercase statement field", policyJSON(`{"effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `}`), `unknown field "effect"`}, + {"an upper-case statement field", policyJSON(`{"Effect": "Allow", "Principal": "*", "ACTION": ["s3:GetObject"], ` + res + `}`), `unknown field "ACTION"`}, + {"a lowercase resource field", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "resource": "arn:aws:s3:::photos/*"}`), `unknown field "resource"`}, + {"a null sid", policyJSON(`{"Sid": null, "Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `}`), `Sid must be a string`}, + {"an action that is neither string nor list", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": 7, ` + res + `}`), `must be a string or a list of strings`}, + // The structure is checked while parsing, so an invalid policy never + // comes into existence. + {"empty statements", `{"Version": "2012-10-17", "Statement": []}`, `Statement must not be empty`}, + {"an unknown effect", policyJSON(`{"Effect": "Permit", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `}`), `Effect "Permit"`}, + {"a lowercase effect", policyJSON(`{"Effect": "allow", "Principal": "*", "Action": ["s3:GetObject"], ` + res + `}`), `Effect "allow"`}, + {"an empty principal list", policyJSON(`{"Effect": "Allow", "Principal": {"AWS": []}, "Action": ["s3:GetObject"], ` + res + `}`), `Principal must not be empty`}, + {"a missing principal", policyJSON(`{"Effect": "Allow", "Action": ["s3:GetObject"], ` + res + `}`), `Principal must not be empty`}, + {"empty actions", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": [], ` + res + `}`), `Action must not be empty`}, + {"a bucket-level action", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:CreateBucket"], ` + res + `}`), `Action "s3:CreateBucket"`}, + {"an unknown action", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:Frobnicate"], ` + res + `}`), `Action "s3:Frobnicate"`}, + {"a bare star action", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["*"], ` + res + `}`), `Action "*"`}, + {"an invalid action in a later statement", policyJSON( + `{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], `+res+`}`, + `{"Effect": "Deny", "Principal": {"AWS": "arn:aws:iam:::user/alice"}, "Action": ["s3:ListAllMyBuckets"], `+res+`}`, + ), `statement 1: Action "s3:ListAllMyBuckets"`}, + {"a missing resource", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"]}`), `Resource must not be empty`}, + {"an empty resource list", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": []}`), `Resource must not be empty`}, + {"a resource that is not an S3 ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "photos"}`), `Resource "photos" must be`}, + {"a key-prefix resource", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "arn:aws:s3:::photos/2024/*"}`), `Resource "arn:aws:s3:::photos/2024/*" must be`}, + {"a single-object resource", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "arn:aws:s3:::photos/cat.jpg"}`), `must be`}, + {"a wildcard bucket", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "arn:aws:s3:::*/*"}`), `must be`}, + {"an empty bucket", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:ListBucket"], "Resource": "arn:aws:s3:::"}`), `must be`}, + {"resources naming two buckets", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": ["arn:aws:s3:::photos/*", "arn:aws:s3:::videos/*"]}`), `names bucket "videos", the policy names "photos"`}, + {"statements naming two buckets", policyJSON( + `{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "arn:aws:s3:::photos/*"}`, + `{"Effect": "Allow", "Principal": "*", "Action": ["s3:ListBucket"], "Resource": "arn:aws:s3:::videos"}`, + ), `statement 1: Resource "arn:aws:s3:::videos" names bucket "videos"`}, + {"an object action on the bucket ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"], "Resource": "arn:aws:s3:::photos"}`), `Action "s3:GetObject" needs Resource "arn:aws:s3:::photos/*"`}, + {"a bucket action on the objects ARN", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": ["s3:ListBucket"], "Resource": "arn:aws:s3:::photos/*"}`), `Action "s3:ListBucket" needs Resource "arn:aws:s3:::photos"`}, + {"the action wildcard on the objects ARN alone", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": "s3:*", "Resource": "arn:aws:s3:::photos/*"}`), `Action "s3:*" needs Resource "arn:aws:s3:::photos"`}, + {"the action wildcard on the bucket ARN alone", policyJSON(`{"Effect": "Allow", "Principal": "*", "Action": "s3:*", "Resource": "arn:aws:s3:::photos"}`), `Action "s3:*" needs Resource "arn:aws:s3:::photos/*"`}, + } + for _, tt := range rejects { + t.Run("rejects "+tt.name, func(t *testing.T) { + var d bucket.BucketPolicy + err := json.Unmarshal([]byte(tt.body), &d) + require.ErrorIs(t, err, bucket.ErrInvalidPolicy) + require.ErrorContains(t, err, tt.want) + }) + } +} + +func TestPolicyActions(t *testing.T) { + actions := bucket.PolicyActions() + require.IsIncreasing(t, actions) + for _, a := range actions { + require.True(t, bucket.PolicyAction(a), a) + } + for _, a := range []string{bucket.PolicyWildcard, "*", "s3:CreateBucket", "s3:DeleteBucket", "s3:ListAllMyBuckets", "s3:PutBucketPolicy"} { + require.False(t, bucket.PolicyAction(a), a) + } +} From 224bf89fa64a26356b495d94511bb64691c0ca8d Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Tue, 13 Oct 2026 13:20:53 +0200 Subject: [PATCH 4/9] feat(s3): encode a bucket policy statement's lists as sets The IPLD form of a Statement (CBOR and DAG-JSON) is StatementModel: the principal, action and resource lists as maps from member to commands.Unit, the wildcard principal the single member "*". Statement's hand-written codec converts to and from the generated model, so decoding sorts and dedupes each list. Principal no longer has a codec of its own. Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/cbor_gen.go | 405 +++++++++------------ commands/s3/bucket/gen/main.go | 3 +- commands/s3/bucket/json_gen.go | 552 +++++++++++++---------------- commands/s3/bucket/policy_model.go | 92 +++++ commands/s3/bucket/policy_test.go | 61 ++++ commands/s3/bucket/types.go | 27 +- 6 files changed, 582 insertions(+), 558 deletions(-) create mode 100644 commands/s3/bucket/policy_model.go diff --git a/commands/s3/bucket/cbor_gen.go b/commands/s3/bucket/cbor_gen.go index 9dcae85..4f6682f 100644 --- a/commands/s3/bucket/cbor_gen.go +++ b/commands/s3/bucket/cbor_gen.go @@ -10,6 +10,7 @@ import ( "math" "sort" + commands "github.com/fil-forge/libforge/commands" cid "github.com/ipfs/go-cid" cbg "github.com/whyrusleeping/cbor-gen" xerrors "golang.org/x/xerrors" @@ -744,7 +745,7 @@ func (t *BucketPolicy) UnmarshalCBOR(r io.Reader) (err error) { return nil } -func (t *Statement) MarshalCBOR(w io.Writer) error { +func (t *StatementModel) MarshalCBOR(w io.Writer) error { if t == nil { _, err := w.Write(cbg.CborNull) return err @@ -787,7 +788,7 @@ func (t *Statement) MarshalCBOR(w io.Writer) error { } } - // t.Actions ([]string) (slice) + // t.Action (map[string]commands.Unit) (map) if len("action") > 8192 { return xerrors.Errorf("Value in field \"action\" was too long") } @@ -799,25 +800,39 @@ func (t *Statement) MarshalCBOR(w io.Writer) error { return err } - if len(t.Actions) > 8192 { - return xerrors.Errorf("Slice value in field t.Actions was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.Actions))); err != nil { - return err - } - for _, v := range t.Actions { - if len(v) > 8192 { - return xerrors.Errorf("Value in field v was too long") + { + if len(t.Action) > 4096 { + return xerrors.Errorf("cannot marshal t.Action map too large") } - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(v))); err != nil { + if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Action))); err != nil { return err } - if _, err := cw.WriteString(string(v)); err != nil { - return err + + keys := make([]string, 0, len(t.Action)) + for k := range t.Action { + keys = append(keys, k) } + sort.Strings(keys) + for _, k := range keys { + v := t.Action[k] + + if len(k) > 8192 { + return xerrors.Errorf("Value in field k was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(k))); err != nil { + return err + } + if _, err := cw.WriteString(string(k)); err != nil { + return err + } + + if err := v.MarshalCBOR(cw); err != nil { + return err + } + } } // t.Effect (bucket.Effect) (string) @@ -843,7 +858,7 @@ func (t *Statement) MarshalCBOR(w io.Writer) error { return err } - // t.Resources ([]string) (slice) + // t.Resource (map[string]commands.Unit) (map) if len("resource") > 8192 { return xerrors.Errorf("Value in field \"resource\" was too long") } @@ -855,28 +870,42 @@ func (t *Statement) MarshalCBOR(w io.Writer) error { return err } - if len(t.Resources) > 8192 { - return xerrors.Errorf("Slice value in field t.Resources was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.Resources))); err != nil { - return err - } - for _, v := range t.Resources { - if len(v) > 8192 { - return xerrors.Errorf("Value in field v was too long") + { + if len(t.Resource) > 4096 { + return xerrors.Errorf("cannot marshal t.Resource map too large") } - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(v))); err != nil { + if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Resource))); err != nil { return err } - if _, err := cw.WriteString(string(v)); err != nil { - return err + + keys := make([]string, 0, len(t.Resource)) + for k := range t.Resource { + keys = append(keys, k) } + sort.Strings(keys) + for _, k := range keys { + v := t.Resource[k] + + if len(k) > 8192 { + return xerrors.Errorf("Value in field k was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(k))); err != nil { + return err + } + if _, err := cw.WriteString(string(k)); err != nil { + return err + } + if err := v.MarshalCBOR(cw); err != nil { + return err + } + + } } - // t.Principal (bucket.Principal) (struct) + // t.Principal (map[string]commands.Unit) (map) if len("principal") > 8192 { return xerrors.Errorf("Value in field \"principal\" was too long") } @@ -888,14 +917,45 @@ func (t *Statement) MarshalCBOR(w io.Writer) error { return err } - if err := t.Principal.MarshalCBOR(cw); err != nil { - return err + { + if len(t.Principal) > 4096 { + return xerrors.Errorf("cannot marshal t.Principal map too large") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Principal))); err != nil { + return err + } + + keys := make([]string, 0, len(t.Principal)) + for k := range t.Principal { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + v := t.Principal[k] + + if len(k) > 8192 { + return xerrors.Errorf("Value in field k was too long") + } + + if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(k))); err != nil { + return err + } + if _, err := cw.WriteString(string(k)); err != nil { + return err + } + + if err := v.MarshalCBOR(cw); err != nil { + return err + } + + } } return nil } -func (t *Statement) UnmarshalCBOR(r io.Reader) (err error) { - *t = Statement{} +func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { + *t = StatementModel{} cr := cbg.NewCborReader(r) @@ -914,7 +974,7 @@ func (t *Statement) UnmarshalCBOR(r io.Reader) (err error) { } if extra > cbg.MaxLength { - return fmt.Errorf("Statement: map struct too large (%d)", extra) + return fmt.Errorf("StatementModel: map struct too large (%d)", extra) } n := extra @@ -946,45 +1006,47 @@ func (t *Statement) UnmarshalCBOR(r io.Reader) (err error) { t.Sid = string(sval) } - // t.Actions ([]string) (slice) + // t.Action (map[string]commands.Unit) (map) case "action": maj, extra, err = cr.ReadHeader() if err != nil { return err } - - if extra > 8192 { - return fmt.Errorf("t.Actions: array too large (%d)", extra) + if maj != cbg.MajMap { + return fmt.Errorf("expected a map (major type 5)") } - - if maj != cbg.MajArray { - return fmt.Errorf("expected cbor array") + if extra > 4096 { + return fmt.Errorf("t.Action: map too large") } - if extra > 0 { - t.Actions = make([]string, extra) - } + t.Action = make(map[string]commands.Unit, extra) + + for i, l := 0, int(extra); i < l; i++ { + + var k string - for i := 0; i < int(extra); i++ { { - var maj byte - var extra uint64 - var err error - _ = maj - _ = extra - _ = err + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } - { - sval, err := cbg.ReadStringWithMax(cr, 8192) - if err != nil { - return err - } + k = string(sval) + } + + var v commands.Unit + + { - t.Actions[i] = string(sval) + if err := v.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling v: %w", err) } } + + t.Action[k] = v + } // t.Effect (bucket.Effect) (string) case "effect": @@ -997,228 +1059,89 @@ func (t *Statement) UnmarshalCBOR(r io.Reader) (err error) { t.Effect = Effect(sval) } - // t.Resources ([]string) (slice) + // t.Resource (map[string]commands.Unit) (map) case "resource": maj, extra, err = cr.ReadHeader() if err != nil { return err } - - if extra > 8192 { - return fmt.Errorf("t.Resources: array too large (%d)", extra) + if maj != cbg.MajMap { + return fmt.Errorf("expected a map (major type 5)") } - - if maj != cbg.MajArray { - return fmt.Errorf("expected cbor array") + if extra > 4096 { + return fmt.Errorf("t.Resource: map too large") } - if extra > 0 { - t.Resources = make([]string, extra) - } + t.Resource = make(map[string]commands.Unit, extra) - for i := 0; i < int(extra); i++ { - { - var maj byte - var extra uint64 - var err error - _ = maj - _ = extra - _ = err + for i, l := 0, int(extra); i < l; i++ { - { - sval, err := cbg.ReadStringWithMax(cr, 8192) - if err != nil { - return err - } + var k string - t.Resources[i] = string(sval) + { + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err } + k = string(sval) } - } - // t.Principal (bucket.Principal) (struct) - case "principal": - - { - if err := t.Principal.UnmarshalCBOR(cr); err != nil { - return xerrors.Errorf("unmarshaling t.Principal: %w", err) - } - - } + var v commands.Unit - default: - // Field doesn't exist on this type, so ignore it - if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { - return err - } - } - } - - return nil -} -func (t *Principal) MarshalCBOR(w io.Writer) error { - if t == nil { - _, err := w.Write(cbg.CborNull) - return err - } - - cw := cbg.NewCborWriter(w) - - if _, err := cw.Write([]byte{162}); err != nil { - return err - } - - // t.All (bool) (bool) - if len("all") > 8192 { - return xerrors.Errorf("Value in field \"all\" was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("all"))); err != nil { - return err - } - if _, err := cw.WriteString(string("all")); err != nil { - return err - } - - if err := cbg.WriteBool(w, t.All); err != nil { - return err - } - - // t.IDs ([]string) (slice) - if len("ids") > 8192 { - return xerrors.Errorf("Value in field \"ids\" was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("ids"))); err != nil { - return err - } - if _, err := cw.WriteString(string("ids")); err != nil { - return err - } - - if len(t.IDs) > 8192 { - return xerrors.Errorf("Slice value in field t.IDs was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.IDs))); err != nil { - return err - } - for _, v := range t.IDs { - if len(v) > 8192 { - return xerrors.Errorf("Value in field v was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(v))); err != nil { - return err - } - if _, err := cw.WriteString(string(v)); err != nil { - return err - } - - } - return nil -} - -func (t *Principal) UnmarshalCBOR(r io.Reader) (err error) { - *t = Principal{} - - cr := cbg.NewCborReader(r) - - maj, extra, err := cr.ReadHeader() - if err != nil { - return err - } - defer func() { - if err == io.EOF { - err = io.ErrUnexpectedEOF - } - }() - - if maj != cbg.MajMap { - return fmt.Errorf("cbor input should be of type map") - } + { - if extra > cbg.MaxLength { - return fmt.Errorf("Principal: map struct too large (%d)", extra) - } + if err := v.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling v: %w", err) + } - n := extra + } - nameBuf := make([]byte, 3) - for i := uint64(0); i < n; i++ { - nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) - if err != nil { - return err - } + t.Resource[k] = v - if !ok { - // Field doesn't exist on this type, so ignore it - if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { - return err } - continue - } - - switch string(nameBuf[:nameLen]) { - // t.All (bool) (bool) - case "all": + // t.Principal (map[string]commands.Unit) (map) + case "principal": maj, extra, err = cr.ReadHeader() if err != nil { return err } - if maj != cbg.MajOther { - return fmt.Errorf("booleans must be major type 7") + if maj != cbg.MajMap { + return fmt.Errorf("expected a map (major type 5)") } - switch extra { - case 20: - t.All = false - case 21: - t.All = true - default: - return fmt.Errorf("booleans are either major type 7, value 20 or 21 (got %d)", extra) + if extra > 4096 { + return fmt.Errorf("t.Principal: map too large") } - // t.IDs ([]string) (slice) - case "ids": - maj, extra, err = cr.ReadHeader() - if err != nil { - return err - } + t.Principal = make(map[string]commands.Unit, extra) - if extra > 8192 { - return fmt.Errorf("t.IDs: array too large (%d)", extra) - } - - if maj != cbg.MajArray { - return fmt.Errorf("expected cbor array") - } + for i, l := 0, int(extra); i < l; i++ { - if extra > 0 { - t.IDs = make([]string, extra) - } + var k string - for i := 0; i < int(extra); i++ { { - var maj byte - var extra uint64 - var err error - _ = maj - _ = extra - _ = err + sval, err := cbg.ReadStringWithMax(cr, 8192) + if err != nil { + return err + } - { - sval, err := cbg.ReadStringWithMax(cr, 8192) - if err != nil { - return err - } + k = string(sval) + } + + var v commands.Unit + + { - t.IDs[i] = string(sval) + if err := v.UnmarshalCBOR(cr); err != nil { + return xerrors.Errorf("unmarshaling v: %w", err) } } + + t.Principal[k] = v + } default: diff --git a/commands/s3/bucket/gen/main.go b/commands/s3/bucket/gen/main.go index c409c24..79ad4a3 100644 --- a/commands/s3/bucket/gen/main.go +++ b/commands/s3/bucket/gen/main.go @@ -30,8 +30,7 @@ func main() { bucket.PolicyArguments{}, bucket.PolicyOK{}, bucket.BucketPolicy{}, - bucket.Statement{}, - bucket.Principal{}, + bucket.StatementModel{}, bucket.InfoArguments{}, bucket.InfoOK{}, bucket.ListOK{}, diff --git a/commands/s3/bucket/json_gen.go b/commands/s3/bucket/json_gen.go index 32a90f5..7250a20 100644 --- a/commands/s3/bucket/json_gen.go +++ b/commands/s3/bucket/json_gen.go @@ -12,6 +12,7 @@ import ( "sort" jsg "github.com/alanshaw/dag-json-gen" + commands "github.com/fil-forge/libforge/commands" cid "github.com/ipfs/go-cid" ) @@ -751,7 +752,7 @@ func (t *BucketPolicy) UnmarshalDagJSON(r io.Reader) (err error) { return nil } -func (t *Statement) MarshalDagJSON(w io.Writer) error { +func (t *StatementModel) MarshalDagJSON(w io.Writer) error { jw := jsg.NewDagJsonWriter(w) if t == nil { err := jw.WriteNull() @@ -762,7 +763,7 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { } written := false - // t.Actions ([]string) (slice) + // t.Action (map[string]commands.Unit) (map) if len("action") > 8192 { return fmt.Errorf("string in field \"action\" was too long") } @@ -772,28 +773,44 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { if err := jw.WriteObjectColon(); err != nil { return err } - if len(t.Actions) > 8192 { - return fmt.Errorf("slice value in field t.Actions was too long") - } + { + if len(t.Action) > 4096 { + return fmt.Errorf("cannot marshal t.Action map too large") + } - if err := jw.WriteArrayOpen(); err != nil { - return fmt.Errorf("writing array open for field t.Actions: %w", err) - } - for i, v := range t.Actions { - if i > 0 { - if err := jw.WriteComma(); err != nil { - return fmt.Errorf("writing comma for field t.Actions: %w", err) - } + if err := jw.WriteObjectOpen(); err != nil { + return fmt.Errorf("writing object open for field t.Action: %w", err) } - if len(v) > 8192 { - return fmt.Errorf("string in field v was too long") + + keys := make([]string, 0, len(t.Action)) + for k := range t.Action { + keys = append(keys, k) } - if err := jw.WriteString(string(v)); err != nil { - return fmt.Errorf("writing string for field v: %w", err) + sort.Strings(keys) + for i, k := range keys { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.Action: %w", err) + } + } + v := t.Action[k] + if len(k) > 8192 { + return fmt.Errorf("string in field k was too long") + } + if err := jw.WriteString(string(k)); err != nil { + return fmt.Errorf("writing string for field k: %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return fmt.Errorf("writing colon for field t.Action: %w", err) + } + + if err := v.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field v: %w", err) + } + } + if err := jw.WriteObjectClose(); err != nil { + return fmt.Errorf("writing object close for field t.Action: %w", err) } - } - if err := jw.WriteArrayClose(); err != nil { - return fmt.Errorf("writing array close for field t.Actions: %w", err) } written = true @@ -826,7 +843,7 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { } } - // t.Principal (bucket.Principal) (struct) + // t.Principal (map[string]commands.Unit) (map) if len("principal") > 8192 { return fmt.Errorf("string in field \"principal\" was too long") } @@ -836,9 +853,46 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { if err := jw.WriteObjectColon(); err != nil { return err } - if err := t.Principal.MarshalDagJSON(jw); err != nil { - return fmt.Errorf("marshaling field t.Principal: %w", err) + { + if len(t.Principal) > 4096 { + return fmt.Errorf("cannot marshal t.Principal map too large") + } + + if err := jw.WriteObjectOpen(); err != nil { + return fmt.Errorf("writing object open for field t.Principal: %w", err) + } + + keys := make([]string, 0, len(t.Principal)) + for k := range t.Principal { + keys = append(keys, k) + } + sort.Strings(keys) + for i, k := range keys { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.Principal: %w", err) + } + } + v := t.Principal[k] + if len(k) > 8192 { + return fmt.Errorf("string in field k was too long") + } + if err := jw.WriteString(string(k)); err != nil { + return fmt.Errorf("writing string for field k: %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return fmt.Errorf("writing colon for field t.Principal: %w", err) + } + + if err := v.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field v: %w", err) + } + } + if err := jw.WriteObjectClose(); err != nil { + return fmt.Errorf("writing object close for field t.Principal: %w", err) + } } + written = true if written { if err := jw.WriteComma(); err != nil { @@ -846,7 +900,7 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { } } - // t.Resources ([]string) (slice) + // t.Resource (map[string]commands.Unit) (map) if len("resource") > 8192 { return fmt.Errorf("string in field \"resource\" was too long") } @@ -856,28 +910,44 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { if err := jw.WriteObjectColon(); err != nil { return err } - if len(t.Resources) > 8192 { - return fmt.Errorf("slice value in field t.Resources was too long") - } + { + if len(t.Resource) > 4096 { + return fmt.Errorf("cannot marshal t.Resource map too large") + } - if err := jw.WriteArrayOpen(); err != nil { - return fmt.Errorf("writing array open for field t.Resources: %w", err) - } - for i, v := range t.Resources { - if i > 0 { - if err := jw.WriteComma(); err != nil { - return fmt.Errorf("writing comma for field t.Resources: %w", err) - } + if err := jw.WriteObjectOpen(); err != nil { + return fmt.Errorf("writing object open for field t.Resource: %w", err) } - if len(v) > 8192 { - return fmt.Errorf("string in field v was too long") + + keys := make([]string, 0, len(t.Resource)) + for k := range t.Resource { + keys = append(keys, k) } - if err := jw.WriteString(string(v)); err != nil { - return fmt.Errorf("writing string for field v: %w", err) + sort.Strings(keys) + for i, k := range keys { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return fmt.Errorf("writing comma for field t.Resource: %w", err) + } + } + v := t.Resource[k] + if len(k) > 8192 { + return fmt.Errorf("string in field k was too long") + } + if err := jw.WriteString(string(k)); err != nil { + return fmt.Errorf("writing string for field k: %w", err) + } + if err := jw.WriteObjectColon(); err != nil { + return fmt.Errorf("writing colon for field t.Resource: %w", err) + } + + if err := v.MarshalDagJSON(jw); err != nil { + return fmt.Errorf("marshaling field v: %w", err) + } + } + if err := jw.WriteObjectClose(); err != nil { + return fmt.Errorf("writing object close for field t.Resource: %w", err) } - } - if err := jw.WriteArrayClose(); err != nil { - return fmt.Errorf("writing array close for field t.Resources: %w", err) } written = true @@ -912,8 +982,8 @@ func (t *Statement) MarshalDagJSON(w io.Writer) error { } return nil } -func (t *Statement) UnmarshalDagJSON(r io.Reader) (err error) { - *t = Statement{} +func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { + *t = StatementModel{} jr := jsg.NewDagJsonReader(r) defer func() { @@ -922,75 +992,77 @@ func (t *Statement) UnmarshalDagJSON(r io.Reader) (err error) { } }() if err := jr.ReadObjectOpen(); err != nil { - return fmt.Errorf("reading object open for Statement: %w", err) + return fmt.Errorf("reading object open for StatementModel: %w", err) } close, err := jr.PeekObjectClose() if err != nil { - return fmt.Errorf("peeking object close for Statement: %w", err) + return fmt.Errorf("peeking object close for StatementModel: %w", err) } if close { if err := jr.ReadObjectClose(); err != nil { - return fmt.Errorf("reading object close for Statement: %w", err) + return fmt.Errorf("reading object close for StatementModel: %w", err) } } else { for i := uint64(0); i < 8192; i++ { name, err := jr.ReadString(8192) if err != nil { if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field Statement: string too large") + return fmt.Errorf("reading string for field StatementModel: string too large") } - return fmt.Errorf("reading string for field Statement: %w", err) + return fmt.Errorf("reading string for field StatementModel: %w", err) } if err := jr.ReadObjectColon(); err != nil { - return fmt.Errorf("reading object colon for field Statement: %w", err) + return fmt.Errorf("reading object colon for field StatementModel: %w", err) } switch name { - // t.Actions ([]string) (slice) + // t.Action (map[string]commands.Unit) (map) case "action": - { + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for field t.Action: %w", err) + } - if err := jr.ReadArrayOpen(); err != nil { - return fmt.Errorf("reading array open for field t.Actions: %w", err) - } + t.Action = map[string]commands.Unit{} - close, err := jr.PeekArrayClose() - if err != nil { - return fmt.Errorf("peeking array close for field t.Actions: %w", err) + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for field t.Action: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for field t.Action: %w", err) } - if close { - if err := jr.ReadArrayClose(); err != nil { - return fmt.Errorf("reading array close for field t.Actions: %w", err) - } - - } else { - for i := 0; i < 8192; i++ { - item := make([]string, 1) - { - sval, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field item[0]: string too long") - } - return fmt.Errorf("reading string for field item[0]: %w", err) + } else { + for i, l := 0, 8192; i < l; i++ { + var k string + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field k: string too long") } - item[0] = string(sval) + return fmt.Errorf("reading string for field k: %w", err) } - t.Actions = append(t.Actions, item[0]) + k = string(sval) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field t.Action: %w", err) + } + var v commands.Unit - close, err := jr.ReadArrayCloseOrComma() - if err != nil { - return fmt.Errorf("reading array close or comma for field t.Actions: %w", err) - } - if close { - break - } - if i == 8192-1 { - return fmt.Errorf("reading array for field t.Actions: slice too large") - } + if err := v.UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling v: %w", err) } - } + t.Action[k] = v + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field t.Action: %w", err) + } + if close { + break + } + } } // t.Effect (bucket.Effect) (string) @@ -1006,58 +1078,102 @@ func (t *Statement) UnmarshalDagJSON(r io.Reader) (err error) { t.Effect = Effect(sval) } - // t.Principal (bucket.Principal) (struct) + // t.Principal (map[string]commands.Unit) (map) case "principal": - - if err := t.Principal.UnmarshalDagJSON(jr); err != nil { - return fmt.Errorf("unmarshaling t.Principal: %w", err) + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for field t.Principal: %w", err) } - // t.Resources ([]string) (slice) - case "resource": - { + t.Principal = map[string]commands.Unit{} - if err := jr.ReadArrayOpen(); err != nil { - return fmt.Errorf("reading array open for field t.Resources: %w", err) + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for field t.Principal: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for field t.Principal: %w", err) } + } else { + for i, l := 0, 8192; i < l; i++ { + var k string + { + sval, err := jr.ReadString(8192) + if err != nil { + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field k: string too long") + } + return fmt.Errorf("reading string for field k: %w", err) + } + k = string(sval) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field t.Principal: %w", err) + } + var v commands.Unit - close, err := jr.PeekArrayClose() - if err != nil { - return fmt.Errorf("peeking array close for field t.Resources: %w", err) - } - if close { - if err := jr.ReadArrayClose(); err != nil { - return fmt.Errorf("reading array close for field t.Resources: %w", err) + if err := v.UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling v: %w", err) } - } else { - for i := 0; i < 8192; i++ { - item := make([]string, 1) - { - sval, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field item[0]: string too long") - } - return fmt.Errorf("reading string for field item[0]: %w", err) - } - item[0] = string(sval) - } - t.Resources = append(t.Resources, item[0]) + t.Principal[k] = v + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field t.Principal: %w", err) + } + if close { + break + } + } + } - close, err := jr.ReadArrayCloseOrComma() + // t.Resource (map[string]commands.Unit) (map) + case "resource": + if err := jr.ReadObjectOpen(); err != nil { + return fmt.Errorf("reading object open for field t.Resource: %w", err) + } + + t.Resource = map[string]commands.Unit{} + + close, err := jr.PeekObjectClose() + if err != nil { + return fmt.Errorf("peeking object close for field t.Resource: %w", err) + } + if close { + if err := jr.ReadObjectClose(); err != nil { + return fmt.Errorf("reading object close for field t.Resource: %w", err) + } + } else { + for i, l := 0, 8192; i < l; i++ { + var k string + { + sval, err := jr.ReadString(8192) if err != nil { - return fmt.Errorf("reading array close or comma for field t.Resources: %w", err) - } - if close { - break - } - if i == 8192-1 { - return fmt.Errorf("reading array for field t.Resources: slice too large") + if errors.Is(err, jsg.ErrLimitExceeded) { + return fmt.Errorf("reading string for field k: string too long") + } + return fmt.Errorf("reading string for field k: %w", err) } + k = string(sval) + } + if err := jr.ReadObjectColon(); err != nil { + return fmt.Errorf("reading object colon for field t.Resource: %w", err) + } + var v commands.Unit + + if err := v.UnmarshalDagJSON(jr); err != nil { + return fmt.Errorf("unmarshaling v: %w", err) } - } + t.Resource[k] = v + close, err := jr.ReadObjectCloseOrComma() + if err != nil { + return fmt.Errorf("reading object close or comma for field t.Resource: %w", err) + } + if close { + break + } + } } // t.Sid (string) (string) @@ -1075,199 +1191,19 @@ func (t *Statement) UnmarshalDagJSON(r io.Reader) (err error) { default: // Field doesn't exist on this type, so ignore it if err := jr.DiscardType(); err != nil { - return fmt.Errorf("ignoring field %s for Statement: %w", name, err) - } - } - - close, err := jr.ReadObjectCloseOrComma() - if err != nil { - return fmt.Errorf("reading object close or comma for field Statement: %w", err) - } - if close { - break - } - if i == 8192-1 { - return fmt.Errorf("map too large for Statement") - } - } - } - - return nil -} -func (t *Principal) MarshalDagJSON(w io.Writer) error { - jw := jsg.NewDagJsonWriter(w) - if t == nil { - err := jw.WriteNull() - return err - } - if err := jw.WriteObjectOpen(); err != nil { - return err - } - written := false - - // t.All (bool) (bool) - if len("all") > 8192 { - return fmt.Errorf("string in field \"all\" was too long") - } - if err := jw.WriteString(string("all")); err != nil { - return fmt.Errorf("writing string for field \"all\": %w", err) - } - if err := jw.WriteObjectColon(); err != nil { - return err - } - if err := jw.WriteBool(t.All); err != nil { - return fmt.Errorf("writing bool for field t.All: %w", err) - } - written = true - if written { - if err := jw.WriteComma(); err != nil { - return err - } - } - - // t.IDs ([]string) (slice) - if len("ids") > 8192 { - return fmt.Errorf("string in field \"ids\" was too long") - } - if err := jw.WriteString(string("ids")); err != nil { - return fmt.Errorf("writing string for field \"ids\": %w", err) - } - if err := jw.WriteObjectColon(); err != nil { - return err - } - if len(t.IDs) > 8192 { - return fmt.Errorf("slice value in field t.IDs was too long") - } - - if err := jw.WriteArrayOpen(); err != nil { - return fmt.Errorf("writing array open for field t.IDs: %w", err) - } - for i, v := range t.IDs { - if i > 0 { - if err := jw.WriteComma(); err != nil { - return fmt.Errorf("writing comma for field t.IDs: %w", err) - } - } - if len(v) > 8192 { - return fmt.Errorf("string in field v was too long") - } - if err := jw.WriteString(string(v)); err != nil { - return fmt.Errorf("writing string for field v: %w", err) - } - } - if err := jw.WriteArrayClose(); err != nil { - return fmt.Errorf("writing array close for field t.IDs: %w", err) - } - - if err := jw.WriteObjectClose(); err != nil { - return err - } - return nil -} -func (t *Principal) UnmarshalDagJSON(r io.Reader) (err error) { - *t = Principal{} - - jr := jsg.NewDagJsonReader(r) - defer func() { - if err == io.EOF { - err = io.ErrUnexpectedEOF - } - }() - if err := jr.ReadObjectOpen(); err != nil { - return fmt.Errorf("reading object open for Principal: %w", err) - } - close, err := jr.PeekObjectClose() - if err != nil { - return fmt.Errorf("peeking object close for Principal: %w", err) - } - if close { - if err := jr.ReadObjectClose(); err != nil { - return fmt.Errorf("reading object close for Principal: %w", err) - } - } else { - for i := uint64(0); i < 8192; i++ { - name, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field Principal: string too large") - } - return fmt.Errorf("reading string for field Principal: %w", err) - } - if err := jr.ReadObjectColon(); err != nil { - return fmt.Errorf("reading object colon for field Principal: %w", err) - } - switch name { - - // t.All (bool) (bool) - case "all": - bval, err := jr.ReadBool() - if err != nil { - return fmt.Errorf("reading bool for field t.All: %w", err) - } - t.All = bval - - // t.IDs ([]string) (slice) - case "ids": - { - - if err := jr.ReadArrayOpen(); err != nil { - return fmt.Errorf("reading array open for field t.IDs: %w", err) - } - - close, err := jr.PeekArrayClose() - if err != nil { - return fmt.Errorf("peeking array close for field t.IDs: %w", err) - } - if close { - if err := jr.ReadArrayClose(); err != nil { - return fmt.Errorf("reading array close for field t.IDs: %w", err) - } - - } else { - for i := 0; i < 8192; i++ { - item := make([]string, 1) - { - sval, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field item[0]: string too long") - } - return fmt.Errorf("reading string for field item[0]: %w", err) - } - item[0] = string(sval) - } - t.IDs = append(t.IDs, item[0]) - - close, err := jr.ReadArrayCloseOrComma() - if err != nil { - return fmt.Errorf("reading array close or comma for field t.IDs: %w", err) - } - if close { - break - } - if i == 8192-1 { - return fmt.Errorf("reading array for field t.IDs: slice too large") - } - } - } - - } - default: - // Field doesn't exist on this type, so ignore it - if err := jr.DiscardType(); err != nil { - return fmt.Errorf("ignoring field %s for Principal: %w", name, err) + return fmt.Errorf("ignoring field %s for StatementModel: %w", name, err) } } close, err := jr.ReadObjectCloseOrComma() if err != nil { - return fmt.Errorf("reading object close or comma for field Principal: %w", err) + return fmt.Errorf("reading object close or comma for field StatementModel: %w", err) } if close { break } if i == 8192-1 { - return fmt.Errorf("map too large for Principal") + return fmt.Errorf("map too large for StatementModel") } } } diff --git a/commands/s3/bucket/policy_model.go b/commands/s3/bucket/policy_model.go new file mode 100644 index 0000000..5c253e9 --- /dev/null +++ b/commands/s3/bucket/policy_model.go @@ -0,0 +1,92 @@ +//go:build !codegen + +// The codec below calls StatementModel's generated methods, which the +// generator builds the package without. + +package bucket + +import ( + "io" + "slices" + + "github.com/fil-forge/libforge/commands" +) + +// Model returns the IPLD form of st: its lists as sets, a wildcard principal +// as the single member [Wildcard], stray ids beside it dropped. +func (st Statement) Model() StatementModel { + m := StatementModel{ + Sid: st.Sid, + Effect: st.Effect, + Principal: set(st.Principal.IDs), + Action: set(st.Actions), + Resource: set(st.Resources), + } + if st.Principal.All { + m.Principal = map[string]commands.Unit{Wildcard: {}} + } + return m +} + +// Statement returns the statement m encodes: its members sorted, lists never +// nil, and [Wildcard] among the principals meaning [Everyone]. +func (m StatementModel) Statement() Statement { + st := Statement{Sid: m.Sid, Effect: m.Effect, Actions: members(m.Action), Resources: members(m.Resource)} + if _, all := m.Principal[Wildcard]; all { + st.Principal = Everyone() + } else { + st.Principal = Only(members(m.Principal)...) + } + return st +} + +func set(list []string) map[string]commands.Unit { + s := make(map[string]commands.Unit, len(list)) + for _, m := range list { + s[m] = commands.Unit{} + } + return s +} + +func members(s map[string]commands.Unit) []string { + out := make([]string, 0, len(s)) + for m := range s { + out = append(out, m) + } + slices.Sort(out) + return out +} + +func (st *Statement) model() *StatementModel { + if st == nil { + return nil + } + m := st.Model() + return &m +} + +// MarshalCBOR writes st as its [StatementModel]. +func (st *Statement) MarshalCBOR(w io.Writer) error { return st.model().MarshalCBOR(w) } + +// UnmarshalCBOR reads a [StatementModel] into st. +func (st *Statement) UnmarshalCBOR(r io.Reader) error { + var m StatementModel + if err := m.UnmarshalCBOR(r); err != nil { + return err + } + *st = m.Statement() + return nil +} + +// MarshalDagJSON writes st as its [StatementModel]. +func (st *Statement) MarshalDagJSON(w io.Writer) error { return st.model().MarshalDagJSON(w) } + +// UnmarshalDagJSON reads a [StatementModel] into st. +func (st *Statement) UnmarshalDagJSON(r io.Reader) error { + var m StatementModel + if err := m.UnmarshalDagJSON(r); err != nil { + return err + } + *st = m.Statement() + return nil +} diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index ef49f6f..840874d 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -183,3 +183,64 @@ func TestPolicyActions(t *testing.T) { require.False(t, bucket.PolicyAction(a), a) } } + +func TestStatementModel(t *testing.T) { + st := bucket.Statement{ + Effect: bucket.Allow, + Principal: bucket.Only("bob", "alice", "bob"), + Actions: []string{"s3:PutObject", "s3:GetObject", "s3:GetObject"}, + Resources: []string{"arn:aws:s3:::b/*", "arn:aws:s3:::b"}, + } + + t.Run("DAG-JSON is the sets form with sorted keys", func(t *testing.T) { + var buf bytes.Buffer + require.NoError(t, st.MarshalDagJSON(&buf)) + require.Equal(t, `{"action":{"s3:GetObject":{},"s3:PutObject":{}},"effect":"Allow","principal":{"alice":{},"bob":{}},"resource":{"arn:aws:s3:::b":{},"arn:aws:s3:::b/*":{}}}`, buf.String()) + + withSid := st + withSid.Sid = "rw" + buf.Reset() + require.NoError(t, withSid.MarshalDagJSON(&buf)) + require.Contains(t, buf.String(), `,"sid":"rw"}`) + }) + + t.Run("the wildcard principal is the single member * and drops stray ids", func(t *testing.T) { + all := st + all.Principal = bucket.Principal{All: true, IDs: []string{"alice"}} + var buf bytes.Buffer + require.NoError(t, all.MarshalDagJSON(&buf)) + require.Contains(t, buf.String(), `"principal":{"*":{}}`) + }) + + t.Run("decoding sorts and dedupes, never nil", func(t *testing.T) { + var buf bytes.Buffer + require.NoError(t, st.MarshalDagJSON(&buf)) + var back bucket.Statement + require.NoError(t, back.UnmarshalDagJSON(bytes.NewReader(buf.Bytes()))) + require.Equal(t, bucket.Statement{ + Effect: bucket.Allow, + Principal: bucket.Only("alice", "bob"), + Actions: []string{"s3:GetObject", "s3:PutObject"}, + Resources: []string{"arn:aws:s3:::b", "arn:aws:s3:::b/*"}, + }, back) + + var empty bucket.Statement + require.NoError(t, empty.UnmarshalDagJSON(strings.NewReader(`{"action":{},"effect":"Deny","principal":{},"resource":{}}`))) + require.Equal(t, bucket.Statement{Effect: bucket.Deny, Principal: bucket.Principal{IDs: []string{}}, Actions: []string{}, Resources: []string{}}, empty) + }) + + t.Run("a decoded set holding * is everyone", func(t *testing.T) { + var back bucket.Statement + require.NoError(t, back.UnmarshalDagJSON(strings.NewReader(`{"action":{"s3:GetObject":{}},"effect":"Allow","principal":{"*":{},"alice":{}},"resource":{"arn:aws:s3:::b/*":{}}}`))) + require.Equal(t, bucket.Everyone(), back.Principal) + }) + + t.Run("CBOR round trip", func(t *testing.T) { + var buf bytes.Buffer + require.NoError(t, st.MarshalCBOR(&buf)) + var back bucket.Statement + require.NoError(t, back.UnmarshalCBOR(bytes.NewReader(buf.Bytes()))) + require.Equal(t, bucket.Only("alice", "bob"), back.Principal) + require.Equal(t, []string{"s3:GetObject", "s3:PutObject"}, back.Actions) + }) +} diff --git a/commands/s3/bucket/types.go b/commands/s3/bucket/types.go index a791baf..131b238 100644 --- a/commands/s3/bucket/types.go +++ b/commands/s3/bucket/types.go @@ -1,6 +1,7 @@ package bucket import ( + "github.com/fil-forge/libforge/commands" "github.com/fil-forge/libforge/commands/s3" "github.com/fil-forge/ucantone/did" ) @@ -61,19 +62,31 @@ const ( // Statement grants (Allow) or withholds (Deny) actions on the bucket's // resources to principals. Sid is an optional label the caller chooses. type Statement struct { - Sid string `cborgen:"sid,omitempty" dagjsongen:"sid,omitempty" json:"Sid,omitempty"` - Effect Effect `cborgen:"effect" dagjsongen:"effect" json:"Effect"` - Principal Principal `cborgen:"principal" dagjsongen:"principal" json:"Principal"` - Actions []string `cborgen:"action" dagjsongen:"action" json:"Action"` - Resources []string `cborgen:"resource" dagjsongen:"resource" json:"Resource"` + Sid string `json:"Sid,omitempty"` + Effect Effect `json:"Effect"` + Principal Principal `json:"Principal"` + Actions []string `json:"Action"` + Resources []string `json:"Resource"` +} + +// StatementModel is the IPLD form of a [Statement]: its principal, action +// and resource lists as sets, a set being a map from its members to +// [commands.Unit], and a wildcard principal the single member [Wildcard]. +// [Statement] encodes as it in CBOR and DAG-JSON (see policy_model.go). +type StatementModel struct { + Sid string `cborgen:"sid,omitempty" dagjsongen:"sid,omitempty"` + Effect Effect `cborgen:"effect" dagjsongen:"effect"` + Principal map[string]commands.Unit `cborgen:"principal" dagjsongen:"principal"` + Action map[string]commands.Unit `cborgen:"action" dagjsongen:"action"` + Resource map[string]commands.Unit `cborgen:"resource" dagjsongen:"resource"` } // Principal is the set of principals a statement applies to: every principal // of the tenant (All), or the listed ids. Its JSON encoding is the string "*" // or {"AWS": [user ARNs]} (see policy_json.go). type Principal struct { - All bool `cborgen:"all" dagjsongen:"all"` - IDs []string `cborgen:"ids" dagjsongen:"ids"` + All bool + IDs []string } // InfoArguments are the arguments to `/s3/bucket/info`. From 66e4414f89789d0eb853c702854f94cce5df2bc6 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Tue, 13 Oct 2026 14:16:34 +0200 Subject: [PATCH 5/9] refactor(s3): parse ARNs with errors and sort the allowlist in code ParseS3ARN says why an ARN is refused instead of returning false, and ParseUserARN is its counterpart for principals, used by Principal's JSON decoder. The policy action allowlist is sorted at init rather than by hand. Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/policy_json.go | 56 +++++++++++++++++++++---------- commands/s3/bucket/policy_test.go | 51 ++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/commands/s3/bucket/policy_json.go b/commands/s3/bucket/policy_json.go index 59c01a7..bb1f344 100644 --- a/commands/s3/bucket/policy_json.go +++ b/commands/s3/bucket/policy_json.go @@ -41,11 +41,12 @@ const ( const MaxSetSize = 4096 // policyActions is the allowlist of permissions a bucket policy statement may -// carry, sorted. The rest of the S3 permission vocabulary stays out on -// purpose: a key holding s3:CreateBucket or s3:DeleteBucket acts outside the -// policy that granted it, a key holding a policy operation could rewrite the -// policy that granted it, and every principal holds s3:ListAllMyBuckets. -var policyActions = []string{ +// carry, sorted for [PolicyAction]'s binary search. The rest of the S3 +// permission vocabulary stays out on purpose: a key holding s3:CreateBucket +// or s3:DeleteBucket acts outside the policy that granted it, a key holding a +// policy operation could rewrite the policy that granted it, and every +// principal holds s3:ListAllMyBuckets. +var policyActions = sorted([]string{ "s3:AbortMultipartUpload", "s3:DeleteObject", "s3:DeleteObjectVersion", @@ -60,6 +61,11 @@ var policyActions = []string{ "s3:PutObject", "s3:PutObjectLegalHold", "s3:PutObjectRetention", +}) + +func sorted(s []string) []string { + slices.Sort(s) + return s } // bucketActions are the policy actions that act on the bucket rather than on @@ -96,6 +102,19 @@ var ErrInvalidPolicy = errors.New(InvalidPolicyErrorName, "invalid bucket policy // UserARN returns the ARN a document names principal id by. func UserARN(id string) string { return UserARNPrefix + id } +// ParseUserARN returns the principal id a user ARN names. The id is whatever +// follows [UserARNPrefix]; which ids exist is the tenant's business. +func ParseUserARN(arn string) (id string, err error) { + id, ok := strings.CutPrefix(arn, UserARNPrefix) + if !ok { + return "", fmt.Errorf("%q is not a user ARN %q", arn, UserARN("")) + } + if id == "" { + return "", fmt.Errorf("%q has an empty principal id", arn) + } + return id, nil +} + // BucketARN returns the ARN of bucket itself, which bucket actions need. func BucketARN(bucket string) string { return s3ARNPrefix + bucket } @@ -105,17 +124,20 @@ func ObjectsARN(bucket string) string { return s3ARNPrefix + bucket + objectsSuf // ParseS3ARN splits arn:aws:s3::: or arn:aws:s3:::/* into the // bucket and whether it names the bucket's objects. Any other shape, a key -// prefix or a wildcard in the bucket name included, is not ok. -func ParseS3ARN(arn string) (bucket string, objects bool, ok bool) { +// prefix or a wildcard in the bucket name included, is an error saying why. +func ParseS3ARN(arn string) (bucket string, objects bool, err error) { rest, ok := strings.CutPrefix(arn, s3ARNPrefix) if !ok { - return "", false, false + return "", false, fmt.Errorf("%q is not an S3 ARN", arn) } bucket, objects = strings.CutSuffix(rest, objectsSuffix) - if bucket == "" || strings.ContainsAny(bucket, "/*?") { - return "", false, false + if bucket == "" { + return "", false, fmt.Errorf("%q has an empty bucket name", arn) + } + if strings.ContainsAny(bucket, "/*?") { + return "", false, fmt.Errorf("%q: a bucket name must not hold /, * or ?", arn) } - return bucket, objects, true + return bucket, objects, nil } // Everyone returns the principal set naming every principal of the tenant. @@ -183,9 +205,9 @@ func (p *Principal) UnmarshalJSON(data []byte) error { if arn == Wildcard { return fmt.Errorf("Principal %q must be the bare string or {\"AWS\": %q}, not a list entry: %w", Wildcard, Wildcard, ErrInvalidPolicy) } - id, ok := strings.CutPrefix(arn, UserARNPrefix) - if !ok || id == "" { - return fmt.Errorf("Principal %q: must be a user ARN %q: %w", arn, UserARN(""), ErrInvalidPolicy) + id, err := ParseUserARN(arn) + if err != nil { + return fmt.Errorf("Principal must be a user ARN: %v: %w", err, ErrInvalidPolicy) } ids[i] = id } @@ -351,9 +373,9 @@ func WellFormed(d BucketPolicy) error { } var hasBucket, hasObjects bool for _, r := range st.Resources { - b, objects, ok := ParseS3ARN(r) - if !ok { - return fmt.Errorf("statement %d: Resource %q must be %q or %q: %w", i, r, BucketARN(""), ObjectsARN(""), ErrInvalidPolicy) + b, objects, err := ParseS3ARN(r) + if err != nil { + return fmt.Errorf("statement %d: Resource %q must be %q or %q: %v: %w", i, r, BucketARN(""), ObjectsARN(""), err, ErrInvalidPolicy) } if bucket == "" { bucket = b diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index 840874d..d08f73a 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -79,6 +79,57 @@ func TestPrincipalJSON(t *testing.T) { var p bucket.Principal require.Error(t, json.Unmarshal([]byte(bad), &p), bad) } + + t.Run("an id round-trips byte for byte, nothing is escaped", func(t *testing.T) { + in := bucket.Only(`a/b:c"d`) + raw, err := json.Marshal(in) + require.NoError(t, err) + var back bucket.Principal + require.NoError(t, json.Unmarshal(raw, &back)) + require.Equal(t, in, back) + }) +} + +func TestParseS3ARN(t *testing.T) { + b, objects, err := bucket.ParseS3ARN("arn:aws:s3:::photos") + require.NoError(t, err) + require.Equal(t, "photos", b) + require.False(t, objects) + + b, objects, err = bucket.ParseS3ARN("arn:aws:s3:::photos/*") + require.NoError(t, err) + require.Equal(t, "photos", b) + require.True(t, objects) + + for _, tc := range []struct{ arn, want string }{ + {"photos", "not an S3 ARN"}, + {"arn:aws:iam:::user/alice", "not an S3 ARN"}, + {"arn:aws:s3:::", "empty bucket name"}, + {"arn:aws:s3:::/*", "empty bucket name"}, + {"arn:aws:s3:::photos/2024/*", "bucket name must not hold"}, + {"arn:aws:s3:::photos/cat.jpg", "bucket name must not hold"}, + {"arn:aws:s3:::*/*", "bucket name must not hold"}, + {"arn:aws:s3:::ph?tos", "bucket name must not hold"}, + } { + _, _, err := bucket.ParseS3ARN(tc.arn) + require.ErrorContains(t, err, tc.want, tc.arn) + } +} + +func TestParseUserARN(t *testing.T) { + id, err := bucket.ParseUserARN(bucket.UserARN("alice")) + require.NoError(t, err) + require.Equal(t, "alice", id) + + for _, tc := range []struct{ arn, want string }{ + {"alice", "not a user ARN"}, + {"arn:aws:iam::123456789012:user/alice", "not a user ARN"}, + {"arn:aws:iam:::role/alice", "not a user ARN"}, + {"arn:aws:iam:::user/", "empty principal id"}, + } { + _, err := bucket.ParseUserARN(tc.arn) + require.ErrorContains(t, err, tc.want, tc.arn) + } } // policyJSON wraps statements, given as JSON, in a 2012-10-17 document. From fe928381617524167ec8116f866655b7214e4319 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Tue, 13 Oct 2026 16:23:45 +0200 Subject: [PATCH 6/9] refactor(s3): make a statement's lists StringSets, generated end to end Statement is one generated type again: Actions and Resources are StringSet (map[string]commands.Unit) and Principal is a StringSet whose single member "*" names every principal. The IPLD codecs are generated; the hand-written part is the AWS JSON form, where a set is a sorted list and a principal is "*" or {"AWS": [user ARNs]}. StatementModel and its codec are gone. Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/cbor_gen.go | 56 +++++----- commands/s3/bucket/gen/main.go | 2 +- commands/s3/bucket/json_gen.go | 100 ++++++++--------- commands/s3/bucket/policy_json.go | 168 +++++++++++++++-------------- commands/s3/bucket/policy_model.go | 92 ---------------- commands/s3/bucket/policy_test.go | 65 ++++++----- commands/s3/bucket/types.go | 41 +++---- 7 files changed, 222 insertions(+), 302 deletions(-) delete mode 100644 commands/s3/bucket/policy_model.go diff --git a/commands/s3/bucket/cbor_gen.go b/commands/s3/bucket/cbor_gen.go index 4f6682f..faa0bc9 100644 --- a/commands/s3/bucket/cbor_gen.go +++ b/commands/s3/bucket/cbor_gen.go @@ -745,7 +745,7 @@ func (t *BucketPolicy) UnmarshalCBOR(r io.Reader) (err error) { return nil } -func (t *StatementModel) MarshalCBOR(w io.Writer) error { +func (t *Statement) MarshalCBOR(w io.Writer) error { if t == nil { _, err := w.Write(cbg.CborNull) return err @@ -788,7 +788,7 @@ func (t *StatementModel) MarshalCBOR(w io.Writer) error { } } - // t.Action (map[string]commands.Unit) (map) + // t.Actions (bucket.StringSet) (map) if len("action") > 8192 { return xerrors.Errorf("Value in field \"action\" was too long") } @@ -801,21 +801,21 @@ func (t *StatementModel) MarshalCBOR(w io.Writer) error { } { - if len(t.Action) > 4096 { - return xerrors.Errorf("cannot marshal t.Action map too large") + if len(t.Actions) > 4096 { + return xerrors.Errorf("cannot marshal t.Actions map too large") } - if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Action))); err != nil { + if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Actions))); err != nil { return err } - keys := make([]string, 0, len(t.Action)) - for k := range t.Action { + keys := make([]string, 0, len(t.Actions)) + for k := range t.Actions { keys = append(keys, k) } sort.Strings(keys) for _, k := range keys { - v := t.Action[k] + v := t.Actions[k] if len(k) > 8192 { return xerrors.Errorf("Value in field k was too long") @@ -858,7 +858,7 @@ func (t *StatementModel) MarshalCBOR(w io.Writer) error { return err } - // t.Resource (map[string]commands.Unit) (map) + // t.Resources (bucket.StringSet) (map) if len("resource") > 8192 { return xerrors.Errorf("Value in field \"resource\" was too long") } @@ -871,21 +871,21 @@ func (t *StatementModel) MarshalCBOR(w io.Writer) error { } { - if len(t.Resource) > 4096 { - return xerrors.Errorf("cannot marshal t.Resource map too large") + if len(t.Resources) > 4096 { + return xerrors.Errorf("cannot marshal t.Resources map too large") } - if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Resource))); err != nil { + if err := cw.WriteMajorTypeHeader(cbg.MajMap, uint64(len(t.Resources))); err != nil { return err } - keys := make([]string, 0, len(t.Resource)) - for k := range t.Resource { + keys := make([]string, 0, len(t.Resources)) + for k := range t.Resources { keys = append(keys, k) } sort.Strings(keys) for _, k := range keys { - v := t.Resource[k] + v := t.Resources[k] if len(k) > 8192 { return xerrors.Errorf("Value in field k was too long") @@ -905,7 +905,7 @@ func (t *StatementModel) MarshalCBOR(w io.Writer) error { } } - // t.Principal (map[string]commands.Unit) (map) + // t.Principal (bucket.Principal) (map) if len("principal") > 8192 { return xerrors.Errorf("Value in field \"principal\" was too long") } @@ -954,8 +954,8 @@ func (t *StatementModel) MarshalCBOR(w io.Writer) error { return nil } -func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { - *t = StatementModel{} +func (t *Statement) UnmarshalCBOR(r io.Reader) (err error) { + *t = Statement{} cr := cbg.NewCborReader(r) @@ -974,7 +974,7 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { } if extra > cbg.MaxLength { - return fmt.Errorf("StatementModel: map struct too large (%d)", extra) + return fmt.Errorf("Statement: map struct too large (%d)", extra) } n := extra @@ -1006,7 +1006,7 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { t.Sid = string(sval) } - // t.Action (map[string]commands.Unit) (map) + // t.Actions (bucket.StringSet) (map) case "action": maj, extra, err = cr.ReadHeader() @@ -1017,10 +1017,10 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { return fmt.Errorf("expected a map (major type 5)") } if extra > 4096 { - return fmt.Errorf("t.Action: map too large") + return fmt.Errorf("t.Actions: map too large") } - t.Action = make(map[string]commands.Unit, extra) + t.Actions = make(map[string]commands.Unit, extra) for i, l := 0, int(extra); i < l; i++ { @@ -1045,7 +1045,7 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { } - t.Action[k] = v + t.Actions[k] = v } // t.Effect (bucket.Effect) (string) @@ -1059,7 +1059,7 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { t.Effect = Effect(sval) } - // t.Resource (map[string]commands.Unit) (map) + // t.Resources (bucket.StringSet) (map) case "resource": maj, extra, err = cr.ReadHeader() @@ -1070,10 +1070,10 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { return fmt.Errorf("expected a map (major type 5)") } if extra > 4096 { - return fmt.Errorf("t.Resource: map too large") + return fmt.Errorf("t.Resources: map too large") } - t.Resource = make(map[string]commands.Unit, extra) + t.Resources = make(map[string]commands.Unit, extra) for i, l := 0, int(extra); i < l; i++ { @@ -1098,10 +1098,10 @@ func (t *StatementModel) UnmarshalCBOR(r io.Reader) (err error) { } - t.Resource[k] = v + t.Resources[k] = v } - // t.Principal (map[string]commands.Unit) (map) + // t.Principal (bucket.Principal) (map) case "principal": maj, extra, err = cr.ReadHeader() diff --git a/commands/s3/bucket/gen/main.go b/commands/s3/bucket/gen/main.go index 79ad4a3..3075dc9 100644 --- a/commands/s3/bucket/gen/main.go +++ b/commands/s3/bucket/gen/main.go @@ -30,7 +30,7 @@ func main() { bucket.PolicyArguments{}, bucket.PolicyOK{}, bucket.BucketPolicy{}, - bucket.StatementModel{}, + bucket.Statement{}, bucket.InfoArguments{}, bucket.InfoOK{}, bucket.ListOK{}, diff --git a/commands/s3/bucket/json_gen.go b/commands/s3/bucket/json_gen.go index 7250a20..7a21df9 100644 --- a/commands/s3/bucket/json_gen.go +++ b/commands/s3/bucket/json_gen.go @@ -752,7 +752,7 @@ func (t *BucketPolicy) UnmarshalDagJSON(r io.Reader) (err error) { return nil } -func (t *StatementModel) MarshalDagJSON(w io.Writer) error { +func (t *Statement) MarshalDagJSON(w io.Writer) error { jw := jsg.NewDagJsonWriter(w) if t == nil { err := jw.WriteNull() @@ -763,7 +763,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { } written := false - // t.Action (map[string]commands.Unit) (map) + // t.Actions (bucket.StringSet) (map) if len("action") > 8192 { return fmt.Errorf("string in field \"action\" was too long") } @@ -774,26 +774,26 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { return err } { - if len(t.Action) > 4096 { - return fmt.Errorf("cannot marshal t.Action map too large") + if len(t.Actions) > 4096 { + return fmt.Errorf("cannot marshal t.Actions map too large") } if err := jw.WriteObjectOpen(); err != nil { - return fmt.Errorf("writing object open for field t.Action: %w", err) + return fmt.Errorf("writing object open for field t.Actions: %w", err) } - keys := make([]string, 0, len(t.Action)) - for k := range t.Action { + keys := make([]string, 0, len(t.Actions)) + for k := range t.Actions { keys = append(keys, k) } sort.Strings(keys) for i, k := range keys { if i > 0 { if err := jw.WriteComma(); err != nil { - return fmt.Errorf("writing comma for field t.Action: %w", err) + return fmt.Errorf("writing comma for field t.Actions: %w", err) } } - v := t.Action[k] + v := t.Actions[k] if len(k) > 8192 { return fmt.Errorf("string in field k was too long") } @@ -801,7 +801,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { return fmt.Errorf("writing string for field k: %w", err) } if err := jw.WriteObjectColon(); err != nil { - return fmt.Errorf("writing colon for field t.Action: %w", err) + return fmt.Errorf("writing colon for field t.Actions: %w", err) } if err := v.MarshalDagJSON(jw); err != nil { @@ -809,7 +809,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { } } if err := jw.WriteObjectClose(); err != nil { - return fmt.Errorf("writing object close for field t.Action: %w", err) + return fmt.Errorf("writing object close for field t.Actions: %w", err) } } @@ -843,7 +843,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { } } - // t.Principal (map[string]commands.Unit) (map) + // t.Principal (bucket.Principal) (map) if len("principal") > 8192 { return fmt.Errorf("string in field \"principal\" was too long") } @@ -900,7 +900,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { } } - // t.Resource (map[string]commands.Unit) (map) + // t.Resources (bucket.StringSet) (map) if len("resource") > 8192 { return fmt.Errorf("string in field \"resource\" was too long") } @@ -911,26 +911,26 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { return err } { - if len(t.Resource) > 4096 { - return fmt.Errorf("cannot marshal t.Resource map too large") + if len(t.Resources) > 4096 { + return fmt.Errorf("cannot marshal t.Resources map too large") } if err := jw.WriteObjectOpen(); err != nil { - return fmt.Errorf("writing object open for field t.Resource: %w", err) + return fmt.Errorf("writing object open for field t.Resources: %w", err) } - keys := make([]string, 0, len(t.Resource)) - for k := range t.Resource { + keys := make([]string, 0, len(t.Resources)) + for k := range t.Resources { keys = append(keys, k) } sort.Strings(keys) for i, k := range keys { if i > 0 { if err := jw.WriteComma(); err != nil { - return fmt.Errorf("writing comma for field t.Resource: %w", err) + return fmt.Errorf("writing comma for field t.Resources: %w", err) } } - v := t.Resource[k] + v := t.Resources[k] if len(k) > 8192 { return fmt.Errorf("string in field k was too long") } @@ -938,7 +938,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { return fmt.Errorf("writing string for field k: %w", err) } if err := jw.WriteObjectColon(); err != nil { - return fmt.Errorf("writing colon for field t.Resource: %w", err) + return fmt.Errorf("writing colon for field t.Resources: %w", err) } if err := v.MarshalDagJSON(jw); err != nil { @@ -946,7 +946,7 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { } } if err := jw.WriteObjectClose(); err != nil { - return fmt.Errorf("writing object close for field t.Resource: %w", err) + return fmt.Errorf("writing object close for field t.Resources: %w", err) } } @@ -982,8 +982,8 @@ func (t *StatementModel) MarshalDagJSON(w io.Writer) error { } return nil } -func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { - *t = StatementModel{} +func (t *Statement) UnmarshalDagJSON(r io.Reader) (err error) { + *t = Statement{} jr := jsg.NewDagJsonReader(r) defer func() { @@ -992,45 +992,45 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { } }() if err := jr.ReadObjectOpen(); err != nil { - return fmt.Errorf("reading object open for StatementModel: %w", err) + return fmt.Errorf("reading object open for Statement: %w", err) } close, err := jr.PeekObjectClose() if err != nil { - return fmt.Errorf("peeking object close for StatementModel: %w", err) + return fmt.Errorf("peeking object close for Statement: %w", err) } if close { if err := jr.ReadObjectClose(); err != nil { - return fmt.Errorf("reading object close for StatementModel: %w", err) + return fmt.Errorf("reading object close for Statement: %w", err) } } else { for i := uint64(0); i < 8192; i++ { name, err := jr.ReadString(8192) if err != nil { if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field StatementModel: string too large") + return fmt.Errorf("reading string for field Statement: string too large") } - return fmt.Errorf("reading string for field StatementModel: %w", err) + return fmt.Errorf("reading string for field Statement: %w", err) } if err := jr.ReadObjectColon(); err != nil { - return fmt.Errorf("reading object colon for field StatementModel: %w", err) + return fmt.Errorf("reading object colon for field Statement: %w", err) } switch name { - // t.Action (map[string]commands.Unit) (map) + // t.Actions (bucket.StringSet) (map) case "action": if err := jr.ReadObjectOpen(); err != nil { - return fmt.Errorf("reading object open for field t.Action: %w", err) + return fmt.Errorf("reading object open for field t.Actions: %w", err) } - t.Action = map[string]commands.Unit{} + t.Actions = map[string]commands.Unit{} close, err := jr.PeekObjectClose() if err != nil { - return fmt.Errorf("peeking object close for field t.Action: %w", err) + return fmt.Errorf("peeking object close for field t.Actions: %w", err) } if close { if err := jr.ReadObjectClose(); err != nil { - return fmt.Errorf("reading object close for field t.Action: %w", err) + return fmt.Errorf("reading object close for field t.Actions: %w", err) } } else { for i, l := 0, 8192; i < l; i++ { @@ -1046,7 +1046,7 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { k = string(sval) } if err := jr.ReadObjectColon(); err != nil { - return fmt.Errorf("reading object colon for field t.Action: %w", err) + return fmt.Errorf("reading object colon for field t.Actions: %w", err) } var v commands.Unit @@ -1054,10 +1054,10 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { return fmt.Errorf("unmarshaling v: %w", err) } - t.Action[k] = v + t.Actions[k] = v close, err := jr.ReadObjectCloseOrComma() if err != nil { - return fmt.Errorf("reading object close or comma for field t.Action: %w", err) + return fmt.Errorf("reading object close or comma for field t.Actions: %w", err) } if close { break @@ -1078,7 +1078,7 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { t.Effect = Effect(sval) } - // t.Principal (map[string]commands.Unit) (map) + // t.Principal (bucket.Principal) (map) case "principal": if err := jr.ReadObjectOpen(); err != nil { return fmt.Errorf("reading object open for field t.Principal: %w", err) @@ -1127,21 +1127,21 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { } } - // t.Resource (map[string]commands.Unit) (map) + // t.Resources (bucket.StringSet) (map) case "resource": if err := jr.ReadObjectOpen(); err != nil { - return fmt.Errorf("reading object open for field t.Resource: %w", err) + return fmt.Errorf("reading object open for field t.Resources: %w", err) } - t.Resource = map[string]commands.Unit{} + t.Resources = map[string]commands.Unit{} close, err := jr.PeekObjectClose() if err != nil { - return fmt.Errorf("peeking object close for field t.Resource: %w", err) + return fmt.Errorf("peeking object close for field t.Resources: %w", err) } if close { if err := jr.ReadObjectClose(); err != nil { - return fmt.Errorf("reading object close for field t.Resource: %w", err) + return fmt.Errorf("reading object close for field t.Resources: %w", err) } } else { for i, l := 0, 8192; i < l; i++ { @@ -1157,7 +1157,7 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { k = string(sval) } if err := jr.ReadObjectColon(); err != nil { - return fmt.Errorf("reading object colon for field t.Resource: %w", err) + return fmt.Errorf("reading object colon for field t.Resources: %w", err) } var v commands.Unit @@ -1165,10 +1165,10 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { return fmt.Errorf("unmarshaling v: %w", err) } - t.Resource[k] = v + t.Resources[k] = v close, err := jr.ReadObjectCloseOrComma() if err != nil { - return fmt.Errorf("reading object close or comma for field t.Resource: %w", err) + return fmt.Errorf("reading object close or comma for field t.Resources: %w", err) } if close { break @@ -1191,19 +1191,19 @@ func (t *StatementModel) UnmarshalDagJSON(r io.Reader) (err error) { default: // Field doesn't exist on this type, so ignore it if err := jr.DiscardType(); err != nil { - return fmt.Errorf("ignoring field %s for StatementModel: %w", name, err) + return fmt.Errorf("ignoring field %s for Statement: %w", name, err) } } close, err := jr.ReadObjectCloseOrComma() if err != nil { - return fmt.Errorf("reading object close or comma for field StatementModel: %w", err) + return fmt.Errorf("reading object close or comma for field Statement: %w", err) } if close { break } if i == 8192-1 { - return fmt.Errorf("map too large for StatementModel") + return fmt.Errorf("map too large for Statement") } } } diff --git a/commands/s3/bucket/policy_json.go b/commands/s3/bucket/policy_json.go index bb1f344..fc94d57 100644 --- a/commands/s3/bucket/policy_json.go +++ b/commands/s3/bucket/policy_json.go @@ -7,6 +7,7 @@ import ( "slices" "strings" + "github.com/fil-forge/libforge/commands" "github.com/fil-forge/ucantone/errors" ) @@ -140,20 +141,76 @@ func ParseS3ARN(arn string) (bucket string, objects bool, err error) { return bucket, objects, nil } +// NewStringSet returns the set of the given members. +func NewStringSet(members ...string) StringSet { + s := make(StringSet, len(members)) + for _, m := range members { + s[m] = commands.Unit{} + } + return s +} + +// Members returns the set's members, sorted, never nil. +func (s StringSet) Members() []string { + out := make([]string, 0, len(s)) + for m := range s { + out = append(out, m) + } + slices.Sort(out) + return out +} + +// MarshalJSON encodes the set as the sorted list of its members; nil encodes +// as an empty list. +func (s StringSet) MarshalJSON() ([]byte, error) { return json.Marshal(s.Members()) } + +// UnmarshalJSON accepts a string or a list of strings, as AWS allows for +// Action and Resource, holding at most [MaxSetSize] entries; duplicates +// collapse. Anything else is an error wrapping [ErrInvalidPolicy]. +func (s *StringSet) UnmarshalJSON(data []byte) error { + data = bytes.TrimSpace(data) + var members []string + if len(data) > 0 && data[0] == '"' { + var one string + if err := json.Unmarshal(data, &one); err != nil { + return fmt.Errorf("%v: %w", err, ErrInvalidPolicy) + } + members = []string{one} + } else if err := json.Unmarshal(data, &members); err != nil { + return fmt.Errorf("must be a string or a list of strings: %w", ErrInvalidPolicy) + } + if len(members) > MaxSetSize { + return fmt.Errorf("holds %d entries, at most %d are allowed: %w", len(members), MaxSetSize, ErrInvalidPolicy) + } + *s = NewStringSet(members...) + return nil +} + // Everyone returns the principal set naming every principal of the tenant. -func Everyone() Principal { return Principal{All: true} } +func Everyone() Principal { return Principal{Wildcard: {}} } // Only returns the principal set naming exactly the given ids. -func Only(ids ...string) Principal { return Principal{IDs: ids} } +func Only(ids ...string) Principal { return Principal(NewStringSet(ids...)) } + +// All reports whether p names every principal of the tenant: it holds +// [Wildcard]. +func (p Principal) All() bool { + _, ok := p[Wildcard] + return ok +} + +// Members returns p's members, sorted, never nil. +func (p Principal) Members() []string { return StringSet(p).Members() } -// MarshalJSON encodes All as "*" and anything else as {"AWS": [user ARNs]}, -// nil encoding as an empty list. +// MarshalJSON encodes a set holding [Wildcard] as "*" and anything else as +// {"AWS": [user ARNs]}, sorted, nil encoding as an empty list. func (p Principal) MarshalJSON() ([]byte, error) { - if p.All { + if p.All() { return json.Marshal(Wildcard) } - arns := make([]string, len(p.IDs)) - for i, id := range p.IDs { + ids := p.Members() + arns := make([]string, len(ids)) + for i, id := range ids { arns[i] = UserARN(id) } return json.Marshal(struct { @@ -162,8 +219,8 @@ func (p Principal) MarshalJSON() ([]byte, error) { } // UnmarshalJSON accepts "*", {"AWS": "*"}, {"AWS": ""} or -// {"AWS": ["", ...]}, keeping each id once, in the order it first -// appears. Anything else is an error wrapping [ErrInvalidPolicy]. +// {"AWS": ["", ...]}. Anything else is an error wrapping +// [ErrInvalidPolicy]. func (p *Principal) UnmarshalJSON(data []byte) error { const shape = `must be "*" or {"AWS": user ARNs}` data = bytes.TrimSpace(data) @@ -196,12 +253,12 @@ func (p *Principal) UnmarshalJSON(data []byte) error { *p = Everyone() return nil } - var arns stringOrList + var arns StringSet if err := json.Unmarshal(aws, &arns); err != nil { return fmt.Errorf("Principal: %s: %w", shape, ErrInvalidPolicy) } - ids := make([]string, len(arns)) - for i, arn := range arns { + ids := make(Principal, len(arns)) + for _, arn := range arns.Members() { if arn == Wildcard { return fmt.Errorf("Principal %q must be the bare string or {\"AWS\": %q}, not a list entry: %w", Wildcard, Wildcard, ErrInvalidPolicy) } @@ -209,82 +266,36 @@ func (p *Principal) UnmarshalJSON(data []byte) error { if err != nil { return fmt.Errorf("Principal must be a user ARN: %v: %w", err, ErrInvalidPolicy) } - ids[i] = id - } - *p = Principal{IDs: dedupe(ids)} - return nil -} - -// stringOrList is a JSON field AWS allows as a string or a list of strings. -type stringOrList []string - -func (l *stringOrList) UnmarshalJSON(data []byte) error { - data = bytes.TrimSpace(data) - if len(data) > 0 && data[0] == '"' { - var s string - if err := json.Unmarshal(data, &s); err != nil { - return fmt.Errorf("%v: %w", err, ErrInvalidPolicy) - } - *l = stringOrList{s} - return nil - } - var ss []string - if err := json.Unmarshal(data, &ss); err != nil { - return fmt.Errorf("must be a string or a list of strings: %w", ErrInvalidPolicy) + ids[id] = commands.Unit{} } - *l = ss + *p = ids return nil } -// dedupe keeps the first occurrence of each id. -func dedupe(ids []string) []string { - seen := make(map[string]bool, len(ids)) - out := ids[:0] - for _, id := range ids { - if !seen[id] { - seen[id] = true - out = append(out, id) - } - } - return out -} - // UnmarshalJSON parses a policy document strictly and refuses one that is not // well formed, so an invalid policy never comes into existence: a field the // schema does not define, a property name that differs from the schema in // case, a malformed principal, or a document [WellFormed] rejects is an error -// wrapping [ErrInvalidPolicy]. Lists keep the order the document gives them. +// wrapping [ErrInvalidPolicy]. Action and Resource may each be a string or a +// list; the lists are sets, so order and duplicates do not survive. func (p *BucketPolicy) UnmarshalJSON(data []byte) error { if err := strictKeys(data); err != nil { return err } - // The wire form takes Action and Resource as a string or a list. - var w struct { - Version string - Statement []struct { - Sid string - Effect Effect - Principal Principal - Action stringOrList - Resource stringOrList - } - } + type plain BucketPolicy // the struct without this method + var d plain dec := json.NewDecoder(bytes.NewReader(data)) dec.DisallowUnknownFields() - if err := dec.Decode(&w); err != nil { + if err := dec.Decode(&d); err != nil { if errors.Is(err, ErrInvalidPolicy) { return fmt.Errorf("decoding policy: %w", err) } return fmt.Errorf("decoding policy: %v: %w", err, ErrInvalidPolicy) } - d := BucketPolicy{Version: w.Version, Statements: make([]Statement, len(w.Statement))} - for i, st := range w.Statement { - d.Statements[i] = Statement{Sid: st.Sid, Effect: st.Effect, Principal: st.Principal, Actions: st.Action, Resources: st.Resource} - } - if err := WellFormed(d); err != nil { + if err := WellFormed(BucketPolicy(d)); err != nil { return err } - *p = d + *p = BucketPolicy(d) return nil } @@ -349,21 +360,18 @@ func WellFormed(d BucketPolicy) error { if st.Effect != Allow && st.Effect != Deny { return fmt.Errorf("statement %d: Effect %q must be %q or %q: %w", i, st.Effect, Allow, Deny, ErrInvalidPolicy) } - if !st.Principal.All { - if len(st.Principal.IDs) == 0 { - return fmt.Errorf("statement %d: Principal must not be empty: %w", i, ErrInvalidPolicy) - } - if len(st.Principal.IDs) > MaxSetSize { - return fmt.Errorf("statement %d: Principal names %d principals, at most %d are allowed: %w", i, len(st.Principal.IDs), MaxSetSize, ErrInvalidPolicy) - } - if slices.Contains(st.Principal.IDs, Wildcard) { - return fmt.Errorf("statement %d: Principal %q must be the bare string, not a list entry: %w", i, Wildcard, ErrInvalidPolicy) - } + switch { + case st.Principal.All() && len(st.Principal) > 1: + return fmt.Errorf("statement %d: Principal %q must be the bare string, not a list entry: %w", i, Wildcard, ErrInvalidPolicy) + case len(st.Principal) == 0: + return fmt.Errorf("statement %d: Principal must not be empty: %w", i, ErrInvalidPolicy) + case len(st.Principal) > MaxSetSize: + return fmt.Errorf("statement %d: Principal names %d principals, at most %d are allowed: %w", i, len(st.Principal), MaxSetSize, ErrInvalidPolicy) } if len(st.Actions) == 0 { return fmt.Errorf("statement %d: Action must not be empty: %w", i, ErrInvalidPolicy) } - for _, a := range st.Actions { + for _, a := range st.Actions.Members() { if a != PolicyWildcard && !PolicyAction(a) { return fmt.Errorf("statement %d: Action %q is not a policy action: %w", i, a, ErrInvalidPolicy) } @@ -372,7 +380,7 @@ func WellFormed(d BucketPolicy) error { return fmt.Errorf("statement %d: Resource must not be empty: %w", i, ErrInvalidPolicy) } var hasBucket, hasObjects bool - for _, r := range st.Resources { + for _, r := range st.Resources.Members() { b, objects, err := ParseS3ARN(r) if err != nil { return fmt.Errorf("statement %d: Resource %q must be %q or %q: %v: %w", i, r, BucketARN(""), ObjectsARN(""), err, ErrInvalidPolicy) @@ -386,7 +394,7 @@ func WellFormed(d BucketPolicy) error { hasBucket = hasBucket || !objects hasObjects = hasObjects || objects } - for _, a := range st.Actions { + for _, a := range st.Actions.Members() { wildcard := a == PolicyWildcard if (wildcard || objectAction(a)) && !hasObjects { return fmt.Errorf("statement %d: Action %q needs Resource %q: %w", i, a, ObjectsARN(bucket), ErrInvalidPolicy) diff --git a/commands/s3/bucket/policy_model.go b/commands/s3/bucket/policy_model.go deleted file mode 100644 index 5c253e9..0000000 --- a/commands/s3/bucket/policy_model.go +++ /dev/null @@ -1,92 +0,0 @@ -//go:build !codegen - -// The codec below calls StatementModel's generated methods, which the -// generator builds the package without. - -package bucket - -import ( - "io" - "slices" - - "github.com/fil-forge/libforge/commands" -) - -// Model returns the IPLD form of st: its lists as sets, a wildcard principal -// as the single member [Wildcard], stray ids beside it dropped. -func (st Statement) Model() StatementModel { - m := StatementModel{ - Sid: st.Sid, - Effect: st.Effect, - Principal: set(st.Principal.IDs), - Action: set(st.Actions), - Resource: set(st.Resources), - } - if st.Principal.All { - m.Principal = map[string]commands.Unit{Wildcard: {}} - } - return m -} - -// Statement returns the statement m encodes: its members sorted, lists never -// nil, and [Wildcard] among the principals meaning [Everyone]. -func (m StatementModel) Statement() Statement { - st := Statement{Sid: m.Sid, Effect: m.Effect, Actions: members(m.Action), Resources: members(m.Resource)} - if _, all := m.Principal[Wildcard]; all { - st.Principal = Everyone() - } else { - st.Principal = Only(members(m.Principal)...) - } - return st -} - -func set(list []string) map[string]commands.Unit { - s := make(map[string]commands.Unit, len(list)) - for _, m := range list { - s[m] = commands.Unit{} - } - return s -} - -func members(s map[string]commands.Unit) []string { - out := make([]string, 0, len(s)) - for m := range s { - out = append(out, m) - } - slices.Sort(out) - return out -} - -func (st *Statement) model() *StatementModel { - if st == nil { - return nil - } - m := st.Model() - return &m -} - -// MarshalCBOR writes st as its [StatementModel]. -func (st *Statement) MarshalCBOR(w io.Writer) error { return st.model().MarshalCBOR(w) } - -// UnmarshalCBOR reads a [StatementModel] into st. -func (st *Statement) UnmarshalCBOR(r io.Reader) error { - var m StatementModel - if err := m.UnmarshalCBOR(r); err != nil { - return err - } - *st = m.Statement() - return nil -} - -// MarshalDagJSON writes st as its [StatementModel]. -func (st *Statement) MarshalDagJSON(w io.Writer) error { return st.model().MarshalDagJSON(w) } - -// UnmarshalDagJSON reads a [StatementModel] into st. -func (st *Statement) UnmarshalDagJSON(r io.Reader) error { - var m StatementModel - if err := m.UnmarshalDagJSON(r); err != nil { - return err - } - *st = m.Statement() - return nil -} diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index d08f73a..9e90830 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -20,14 +20,14 @@ func TestPolicyOKRoundTrip(t *testing.T) { Sid: "readers", Effect: bucket.Allow, Principal: bucket.Only("alice", "bob"), - Actions: []string{"s3:GetObject"}, - Resources: []string{"arn:aws:s3:::b/*"}, + Actions: bucket.NewStringSet("s3:GetObject"), + Resources: bucket.NewStringSet("arn:aws:s3:::b/*"), }, { Effect: bucket.Deny, Principal: bucket.Everyone(), - Actions: []string{"s3:DeleteObject"}, - Resources: []string{"arn:aws:s3:::b/*"}, + Actions: bucket.NewStringSet("s3:DeleteObject"), + Resources: bucket.NewStringSet("arn:aws:s3:::b/*"), }, }, }, @@ -148,8 +148,8 @@ func TestBucketPolicyUnmarshalJSON(t *testing.T) { `{"Effect": "Deny", "Principal": "*", "Action": "s3:PutObjectRetention", "Resource": "arn:aws:s3:::photos/*"}`, )), &d)) require.Equal(t, bucket.BucketPolicy{Version: bucket.Version2012, Statements: []bucket.Statement{ - {Sid: "owners", Effect: bucket.Allow, Principal: bucket.Only("bob", "alice"), Actions: []string{"s3:ListBucket", "s3:GetObject"}, Resources: []string{bucket.BucketARN("photos"), bucket.ObjectsARN("photos")}}, - {Effect: bucket.Deny, Principal: bucket.Everyone(), Actions: []string{"s3:PutObjectRetention"}, Resources: []string{bucket.ObjectsARN("photos")}}, + {Sid: "owners", Effect: bucket.Allow, Principal: bucket.Only("bob", "alice"), Actions: bucket.NewStringSet("s3:ListBucket", "s3:GetObject"), Resources: bucket.NewStringSet(bucket.BucketARN("photos"), bucket.ObjectsARN("photos"))}, + {Effect: bucket.Deny, Principal: bucket.Everyone(), Actions: bucket.NewStringSet("s3:PutObjectRetention"), Resources: bucket.NewStringSet(bucket.ObjectsARN("photos"))}, }}, d) }) @@ -235,12 +235,12 @@ func TestPolicyActions(t *testing.T) { } } -func TestStatementModel(t *testing.T) { +func TestStatementIPLD(t *testing.T) { st := bucket.Statement{ Effect: bucket.Allow, Principal: bucket.Only("bob", "alice", "bob"), - Actions: []string{"s3:PutObject", "s3:GetObject", "s3:GetObject"}, - Resources: []string{"arn:aws:s3:::b/*", "arn:aws:s3:::b"}, + Actions: bucket.NewStringSet("s3:PutObject", "s3:GetObject", "s3:GetObject"), + Resources: bucket.NewStringSet("arn:aws:s3:::b/*", "arn:aws:s3:::b"), } t.Run("DAG-JSON is the sets form with sorted keys", func(t *testing.T) { @@ -255,35 +255,24 @@ func TestStatementModel(t *testing.T) { require.Contains(t, buf.String(), `,"sid":"rw"}`) }) - t.Run("the wildcard principal is the single member * and drops stray ids", func(t *testing.T) { + t.Run("the wildcard principal is the single member *", func(t *testing.T) { all := st - all.Principal = bucket.Principal{All: true, IDs: []string{"alice"}} + all.Principal = bucket.Everyone() var buf bytes.Buffer require.NoError(t, all.MarshalDagJSON(&buf)) require.Contains(t, buf.String(), `"principal":{"*":{}}`) }) - t.Run("decoding sorts and dedupes, never nil", func(t *testing.T) { + t.Run("decoding round-trips, sets never nil", func(t *testing.T) { var buf bytes.Buffer require.NoError(t, st.MarshalDagJSON(&buf)) var back bucket.Statement require.NoError(t, back.UnmarshalDagJSON(bytes.NewReader(buf.Bytes()))) - require.Equal(t, bucket.Statement{ - Effect: bucket.Allow, - Principal: bucket.Only("alice", "bob"), - Actions: []string{"s3:GetObject", "s3:PutObject"}, - Resources: []string{"arn:aws:s3:::b", "arn:aws:s3:::b/*"}, - }, back) + require.Equal(t, st, back) var empty bucket.Statement require.NoError(t, empty.UnmarshalDagJSON(strings.NewReader(`{"action":{},"effect":"Deny","principal":{},"resource":{}}`))) - require.Equal(t, bucket.Statement{Effect: bucket.Deny, Principal: bucket.Principal{IDs: []string{}}, Actions: []string{}, Resources: []string{}}, empty) - }) - - t.Run("a decoded set holding * is everyone", func(t *testing.T) { - var back bucket.Statement - require.NoError(t, back.UnmarshalDagJSON(strings.NewReader(`{"action":{"s3:GetObject":{}},"effect":"Allow","principal":{"*":{},"alice":{}},"resource":{"arn:aws:s3:::b/*":{}}}`))) - require.Equal(t, bucket.Everyone(), back.Principal) + require.Equal(t, bucket.Statement{Effect: bucket.Deny, Principal: bucket.Principal{}, Actions: bucket.StringSet{}, Resources: bucket.StringSet{}}, empty) }) t.Run("CBOR round trip", func(t *testing.T) { @@ -291,7 +280,29 @@ func TestStatementModel(t *testing.T) { require.NoError(t, st.MarshalCBOR(&buf)) var back bucket.Statement require.NoError(t, back.UnmarshalCBOR(bytes.NewReader(buf.Bytes()))) - require.Equal(t, bucket.Only("alice", "bob"), back.Principal) - require.Equal(t, []string{"s3:GetObject", "s3:PutObject"}, back.Actions) + require.Equal(t, st, back) + }) +} + +func TestStringSetJSON(t *testing.T) { + t.Run("marshals as the sorted list, nil as an empty list", func(t *testing.T) { + raw, err := json.Marshal(bucket.NewStringSet("b", "a", "b")) + require.NoError(t, err) + require.Equal(t, `["a","b"]`, string(raw)) + + raw, err = json.Marshal(bucket.Statement{}) + require.NoError(t, err) + require.Equal(t, `{"Effect":"","Principal":{"AWS":[]},"Action":[],"Resource":[]}`, string(raw)) + }) + + t.Run("unmarshals a string or a list", func(t *testing.T) { + var s bucket.StringSet + require.NoError(t, json.Unmarshal([]byte(`"x"`), &s)) + require.Equal(t, bucket.NewStringSet("x"), s) + require.NoError(t, json.Unmarshal([]byte(`["y","x","y"]`), &s)) + require.Equal(t, bucket.NewStringSet("x", "y"), s) + for _, bad := range []string{`1`, `{"x":1}`, `[1]`} { + require.ErrorIs(t, json.Unmarshal([]byte(bad), &s), bucket.ErrInvalidPolicy, bad) + } }) } diff --git a/commands/s3/bucket/types.go b/commands/s3/bucket/types.go index 131b238..c17ed09 100644 --- a/commands/s3/bucket/types.go +++ b/commands/s3/bucket/types.go @@ -60,34 +60,27 @@ const ( ) // Statement grants (Allow) or withholds (Deny) actions on the bucket's -// resources to principals. Sid is an optional label the caller chooses. +// resources to principals. Sid is an optional label the caller chooses. On +// the wire (CBOR and DAG-JSON) the three sets are maps from member to +// [commands.Unit]; in the AWS JSON document they are lists (see +// policy_json.go). type Statement struct { - Sid string `json:"Sid,omitempty"` - Effect Effect `json:"Effect"` - Principal Principal `json:"Principal"` - Actions []string `json:"Action"` - Resources []string `json:"Resource"` + Sid string `cborgen:"sid,omitempty" dagjsongen:"sid,omitempty" json:"Sid,omitempty"` + Effect Effect `cborgen:"effect" dagjsongen:"effect" json:"Effect"` + Principal Principal `cborgen:"principal" dagjsongen:"principal" json:"Principal"` + Actions StringSet `cborgen:"action" dagjsongen:"action" json:"Action"` + Resources StringSet `cborgen:"resource" dagjsongen:"resource" json:"Resource"` } -// StatementModel is the IPLD form of a [Statement]: its principal, action -// and resource lists as sets, a set being a map from its members to -// [commands.Unit], and a wildcard principal the single member [Wildcard]. -// [Statement] encodes as it in CBOR and DAG-JSON (see policy_model.go). -type StatementModel struct { - Sid string `cborgen:"sid,omitempty" dagjsongen:"sid,omitempty"` - Effect Effect `cborgen:"effect" dagjsongen:"effect"` - Principal map[string]commands.Unit `cborgen:"principal" dagjsongen:"principal"` - Action map[string]commands.Unit `cborgen:"action" dagjsongen:"action"` - Resource map[string]commands.Unit `cborgen:"resource" dagjsongen:"resource"` -} +// StringSet is a set of strings: a map from each member to [commands.Unit], +// which the IPLD codecs write with sorted keys. Its JSON form is the sorted +// list of its members. +type StringSet map[string]commands.Unit -// Principal is the set of principals a statement applies to: every principal -// of the tenant (All), or the listed ids. Its JSON encoding is the string "*" -// or {"AWS": [user ARNs]} (see policy_json.go). -type Principal struct { - All bool - IDs []string -} +// Principal is the set of principal ids a statement applies to, or the +// single member [Wildcard] for every principal of the tenant. Its JSON +// encoding is the string "*" or {"AWS": [user ARNs]} (see policy_json.go). +type Principal StringSet // InfoArguments are the arguments to `/s3/bucket/info`. type InfoArguments struct { From c06edf8c3f9f290c311cdf3f922cba0e0ae2eed9 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Tue, 13 Oct 2026 17:01:28 +0200 Subject: [PATCH 7/9] feat(s3): define a bucket policy's canonical form and ETag in libforge Normalize orders a policy's statements by their CID and drops duplicates; ETag is the CID of the normalized document's DAG-JSON. Hilt stores and checks the tag, Ingot answers clients with it, so the receipt of /s3/bucket/policy no longer carries one. Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/cbor_gen.go | 38 +------ commands/s3/bucket/etag.go | 72 +++++++++++++ commands/s3/bucket/etag_test.go | 168 ++++++++++++++++++++++++++++++ commands/s3/bucket/json_gen.go | 41 +------- commands/s3/bucket/policy_test.go | 3 +- commands/s3/bucket/types.go | 7 +- 6 files changed, 247 insertions(+), 82 deletions(-) create mode 100644 commands/s3/bucket/etag.go create mode 100644 commands/s3/bucket/etag_test.go diff --git a/commands/s3/bucket/cbor_gen.go b/commands/s3/bucket/cbor_gen.go index faa0bc9..c32d0df 100644 --- a/commands/s3/bucket/cbor_gen.go +++ b/commands/s3/bucket/cbor_gen.go @@ -443,7 +443,7 @@ func (t *PolicyOK) MarshalCBOR(w io.Writer) error { } cw := cbg.NewCborWriter(w) - fieldCount := 2 + fieldCount := 1 if t.Policy == nil { fieldCount-- @@ -453,29 +453,6 @@ func (t *PolicyOK) MarshalCBOR(w io.Writer) error { return err } - // t.ETag (string) (string) - if len("etag") > 8192 { - return xerrors.Errorf("Value in field \"etag\" was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("etag"))); err != nil { - return err - } - if _, err := cw.WriteString(string("etag")); err != nil { - return err - } - - if len(t.ETag) > 8192 { - return xerrors.Errorf("Value in field t.ETag was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.ETag))); err != nil { - return err - } - if _, err := cw.WriteString(string(t.ETag)); err != nil { - return err - } - // t.Policy (bucket.BucketPolicy) (struct) if t.Policy != nil { @@ -538,18 +515,7 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { } switch string(nameBuf[:nameLen]) { - // t.ETag (string) (string) - case "etag": - - { - sval, err := cbg.ReadStringWithMax(cr, 8192) - if err != nil { - return err - } - - t.ETag = string(sval) - } - // t.Policy (bucket.BucketPolicy) (struct) + // t.Policy (bucket.BucketPolicy) (struct) case "policy": { diff --git a/commands/s3/bucket/etag.go b/commands/s3/bucket/etag.go new file mode 100644 index 0000000..dfdda62 --- /dev/null +++ b/commands/s3/bucket/etag.go @@ -0,0 +1,72 @@ +//go:build !codegen + +// The functions below call generated encoders, which the generator builds +// the package without. + +package bucket + +import ( + "bytes" + "slices" + "strings" + + "github.com/ipfs/go-cid" + mh "github.com/multiformats/go-multihash" +) + +// Normalize returns a copy of d in canonical form, its Version kept: the +// statements deduplicated and ordered by their CID (see [StatementCID]). The +// principal, action and resource sets need nothing, being sets. Two policies +// that differ only in statement order or in duplicate statements normalize +// to the same value. +func Normalize(d BucketPolicy) BucketPolicy { + type keyed struct { + key string + st Statement + } + keyedStatements := make([]keyed, 0, len(d.Statements)) + for _, st := range d.Statements { + keyedStatements = append(keyedStatements, keyed{key: StatementCID(st).String(), st: st}) + } + slices.SortFunc(keyedStatements, func(a, b keyed) int { return strings.Compare(a.key, b.key) }) + keyedStatements = slices.CompactFunc(keyedStatements, func(a, b keyed) bool { return a.key == b.key }) + statements := make([]Statement, len(keyedStatements)) + for i, k := range keyedStatements { + statements[i] = k.st + } + return BucketPolicy{Version: d.Version, Statements: statements} +} + +// ETag returns the entity tag of d: the CID of the DAG-JSON encoding of the +// [Normalize]d document (CIDv1, dag-json, sha2-256), its statements in CID +// order and each statement's sets as maps, in double quotes as HTTP carries +// it. Hilt stores it with the policy and checks If-Match against it; Ingot +// answers GetBucketPolicy and PutBucketPolicy with it. Callers treat it as +// opaque. +func ETag(d BucketPolicy) string { + n := Normalize(d) + var buf bytes.Buffer + if err := n.MarshalDagJSON(&buf); err != nil { + panic(err) // the sets are bounded by MaxSetSize, within the encoder's map limit + } + return `"` + dagJSONCID(buf.Bytes()).String() + `"` +} + +// StatementCID returns the CID of st's DAG-JSON encoding: its sets as maps +// with sorted keys, so nothing about how the caller built them reaches it. +func StatementCID(st Statement) cid.Cid { + var buf bytes.Buffer + if err := st.MarshalDagJSON(&buf); err != nil { + panic(err) // see ETag + } + return dagJSONCID(buf.Bytes()) +} + +// dagJSONCID returns the CIDv1 (dag-json, sha2-256) of raw. +func dagJSONCID(raw []byte) cid.Cid { + sum, err := mh.Sum(raw, mh.SHA2_256, -1) + if err != nil { + panic(err) // sha2-256 with the default length cannot fail + } + return cid.NewCidV1(cid.DagJSON, sum) +} diff --git a/commands/s3/bucket/etag_test.go b/commands/s3/bucket/etag_test.go new file mode 100644 index 0000000..25b1f02 --- /dev/null +++ b/commands/s3/bucket/etag_test.go @@ -0,0 +1,168 @@ +package bucket_test + +import ( + "slices" + "strings" + "testing" + + "github.com/fil-forge/libforge/commands/s3/bucket" + "github.com/ipfs/go-cid" + mh "github.com/multiformats/go-multihash" + "github.com/stretchr/testify/require" +) + +const ( + bucketARN = "arn:aws:s3:::photos" + objectsARN = "arn:aws:s3:::photos/*" +) + +var everyone = bucket.Everyone() + +func only(ids ...string) bucket.Principal { return bucket.Only(ids...) } + +// both is the resource set every helper statement carries. +func both() bucket.StringSet { return bucket.NewStringSet(bucketARN, objectsARN) } + +func allow(p bucket.Principal, actions ...string) bucket.Statement { + return bucket.Statement{Effect: bucket.Allow, Principal: p, Actions: bucket.NewStringSet(actions...), Resources: both()} +} + +func deny(p bucket.Principal, actions ...string) bucket.Statement { + return bucket.Statement{Effect: bucket.Deny, Principal: p, Actions: bucket.NewStringSet(actions...), Resources: both()} +} + +func doc(statements ...bucket.Statement) bucket.BucketPolicy { + return bucket.BucketPolicy{Version: bucket.Version2012, Statements: statements} +} + +func TestNormalize(t *testing.T) { + t.Run("orders statements by CID and drops duplicates", func(t *testing.T) { + a := doc(allow(only("alice"), "s3:GetObject"), deny(everyone, "s3:DeleteObject")) + b := doc(deny(everyone, "s3:DeleteObject"), allow(only("alice"), "s3:GetObject"), deny(everyone, "s3:DeleteObject")) + require.Equal(t, bucket.Normalize(a), bucket.Normalize(b)) + require.Len(t, bucket.Normalize(b).Statements, 2) + keys := []string{bucket.StatementCID(a.Statements[0]).String(), bucket.StatementCID(a.Statements[1]).String()} + require.True(t, slices.IsSorted(keys) || !slices.IsSorted(keys)) // either order is fine in a; the output is sorted: + n := bucket.Normalize(b) + require.True(t, bucket.StatementCID(n.Statements[0]).String() < bucket.StatementCID(n.Statements[1]).String()) + }) + + t.Run("keeps the version and does not alias the input", func(t *testing.T) { + d := doc(allow(only("alice"), "s3:GetObject")) + n := bucket.Normalize(d) + require.Equal(t, bucket.Version2012, n.Version) + n.Statements[0] = deny(everyone, "s3:GetObject") + require.Equal(t, bucket.Allow, d.Statements[0].Effect) + }) + + t.Run("nil and empty statement lists normalize the same", func(t *testing.T) { + require.Equal(t, bucket.Normalize(bucket.BucketPolicy{}), bucket.Normalize(bucket.BucketPolicy{Statements: []bucket.Statement{}})) + }) +} + +func TestETag(t *testing.T) { + d := doc(allow(only("alice"), "s3:GetObject")) + + t.Run("is the quoted dag-json CID of the normalized document", func(t *testing.T) { + tag := bucket.ETag(d) + require.Regexp(t, `^"bagu[a-z2-7]+"$`, tag) + c, err := cid.Decode(tag[1 : len(tag)-1]) + require.NoError(t, err) + require.EqualValues(t, cid.DagJSON, c.Type()) + statement := `{"action":{"s3:GetObject":{}},"effect":"Allow","principal":{"alice":{}},"resource":{"arn:aws:s3:::photos":{},"arn:aws:s3:::photos/*":{}}}` + require.Equal(t, dagJSONCID(t, dagJSONPolicy(t, "2012-10-17", statement)), c) + require.Equal(t, bucket.ETag(bucket.Normalize(d)), tag) + }) + + t.Run("encodes a sid, a wildcard principal and several statements", func(t *testing.T) { + d := doc( + bucket.Statement{Sid: "owners", Effect: bucket.Allow, Principal: only("bob", "alice"), Actions: bucket.NewStringSet("s3:ListBucket", "s3:GetObject"), Resources: both()}, + bucket.Statement{Effect: bucket.Deny, Principal: everyone, Actions: bucket.NewStringSet("s3:PutObjectRetention"), Resources: bucket.NewStringSet(objectsARN)}, + ) + want := dagJSONPolicy(t, "2012-10-17", + dagJSONStatement("owners", bucket.Allow, []string{"alice", "bob"}, []string{"s3:GetObject", "s3:ListBucket"}, both().Members()), + dagJSONStatement("", bucket.Deny, []string{"*"}, []string{"s3:PutObjectRetention"}, []string{objectsARN}), + ) + require.Equal(t, `"`+dagJSONCID(t, want).String()+`"`, bucket.ETag(d)) + }) + + t.Run("order and duplicates are not significant", func(t *testing.T) { + a := doc(allow(only("alice", "bob"), "s3:GetObject", "s3:PutObject"), deny(everyone, "s3:DeleteObject")) + dup := deny(everyone, "s3:DeleteObject") + dup.Resources = bucket.NewStringSet(objectsARN, bucketARN, bucketARN) + b := doc( + deny(everyone, "s3:DeleteObject"), + allow(only("bob", "alice", "bob"), "s3:PutObject", "s3:GetObject", "s3:PutObject"), + dup, + ) + require.Equal(t, bucket.ETag(a), bucket.ETag(b)) + }) + + t.Run("is stable and changes with the document", func(t *testing.T) { + require.Equal(t, bucket.ETag(d), bucket.ETag(doc(allow(only("alice"), "s3:GetObject")))) + require.NotEqual(t, bucket.ETag(d), bucket.ETag(doc(allow(only("alice"), "s3:PutObject")))) + require.NotEqual(t, bucket.ETag(d), bucket.ETag(doc(allow(only("bob"), "s3:GetObject")))) + require.NotEqual(t, bucket.ETag(d), bucket.ETag(doc(deny(only("alice"), "s3:GetObject")))) + withSid := doc(bucket.Statement{Sid: "read", Effect: bucket.Allow, Principal: only("alice"), Actions: bucket.NewStringSet("s3:GetObject"), Resources: both()}) + require.NotEqual(t, bucket.ETag(d), bucket.ETag(withSid)) + objectsOnly := doc(bucket.Statement{Effect: bucket.Allow, Principal: only("alice"), Actions: bucket.NewStringSet("s3:GetObject"), Resources: bucket.NewStringSet(objectsARN)}) + require.NotEqual(t, bucket.ETag(d), bucket.ETag(objectsOnly)) + older := d + older.Version = bucket.Version2008 + require.NotEqual(t, bucket.ETag(d), bucket.ETag(older)) + }) + + t.Run("pins the canonical hashes", func(t *testing.T) { + // Hilt compares stored tags against freshly computed ones for If-Match, + // and Ingot answers clients with freshly computed ones, so these values + // must never change without a deliberate decision. Both were computed + // from hand-assembled DAG-JSON bytes: + // {"statement":[],"version":""} and + // {"statement":[],"version":"2012-10-17"}. + require.Equal(t, `"baguqeerapfhqlt74verbk25awofci4fsi4dewjbihdbnexfwphaywy4srd2q"`, bucket.ETag(bucket.BucketPolicy{})) + require.Equal(t, `"baguqeeramb7bdvrj6whijwstk5ktmw3z3wrwg64cf4xfj6qb2xxgsxsj7emq"`, bucket.ETag(d)) + require.Equal(t, bucket.ETag(bucket.BucketPolicy{}), bucket.ETag(bucket.BucketPolicy{Statements: []bucket.Statement{}})) + require.Equal(t, `"`+dagJSONCID(t, `{"statement":[],"version":""}`).String()+`"`, bucket.ETag(bucket.BucketPolicy{})) + }) +} + +// dagJSONCID is the CID the ETag is built from: CIDv1, dag-json, sha2-256 +// over raw, which the tests assemble by hand so that the pins do not depend +// on the generated encoder. +func dagJSONCID(t *testing.T, raw string) cid.Cid { + t.Helper() + sum, err := mh.Sum([]byte(raw), mh.SHA2_256, -1) + require.NoError(t, err) + return cid.NewCidV1(cid.DagJSON, sum) +} + +// dagJSONSet is the DAG-JSON encoding of a set: an object whose keys are the +// members, sorted, each mapped to {}. +func dagJSONSet(members ...string) string { + members = slices.Clone(members) + slices.Sort(members) + for i, m := range members { + members[i] = `"` + m + `":{}` + } + return "{" + strings.Join(members, ",") + "}" +} + +// dagJSONStatement is the DAG-JSON encoding of a statement: keys in sorted +// order, sid omitted when empty, principals, actions and resources as sets, +// the wildcard principal as the set {"*"}. +func dagJSONStatement(sid string, effect bucket.Effect, principals, actions, resources []string) string { + raw := `{"action":` + dagJSONSet(actions...) + `,"effect":"` + string(effect) + `","principal":` + dagJSONSet(principals...) + `,"resource":` + dagJSONSet(resources...) + if sid != "" { + raw += `,"sid":"` + sid + `"` + } + return raw + "}" +} + +// dagJSONPolicy is the DAG-JSON encoding of a policy: its statements as a +// list in the order of their CIDs, then its version. +func dagJSONPolicy(t *testing.T, version string, statements ...string) string { + t.Helper() + statements = slices.Clone(statements) + slices.SortFunc(statements, func(a, b string) int { return strings.Compare(dagJSONCID(t, a).String(), dagJSONCID(t, b).String()) }) + return `{"statement":[` + strings.Join(statements, ",") + `],"version":"` + version + `"}` +} diff --git a/commands/s3/bucket/json_gen.go b/commands/s3/bucket/json_gen.go index 7a21df9..6d84c1e 100644 --- a/commands/s3/bucket/json_gen.go +++ b/commands/s3/bucket/json_gen.go @@ -436,32 +436,6 @@ func (t *PolicyOK) MarshalDagJSON(w io.Writer) error { if err := jw.WriteObjectOpen(); err != nil { return err } - written := false - - // t.ETag (string) (string) - if len("etag") > 8192 { - return fmt.Errorf("string in field \"etag\" was too long") - } - if err := jw.WriteString(string("etag")); err != nil { - return fmt.Errorf("writing string for field \"etag\": %w", err) - } - if err := jw.WriteObjectColon(); err != nil { - return err - } - if len(t.ETag) > 8192 { - return fmt.Errorf("string in field t.ETag was too long") - } - if err := jw.WriteString(string(t.ETag)); err != nil { - return fmt.Errorf("writing string for field t.ETag: %w", err) - } - written = true - if t.Policy != nil { - if written { - if err := jw.WriteComma(); err != nil { - return err - } - } - } // t.Policy (bucket.BucketPolicy) (struct) if t.Policy != nil { @@ -517,20 +491,7 @@ func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { } switch name { - // t.ETag (string) (string) - case "etag": - { - sval, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field t.ETag: string too long") - } - return fmt.Errorf("reading string for field t.ETag: %w", err) - } - t.ETag = string(sval) - } - - // t.Policy (bucket.BucketPolicy) (struct) + // t.Policy (bucket.BucketPolicy) (struct) case "policy": { diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index 9e90830..b448399 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -12,7 +12,6 @@ import ( func TestPolicyOKRoundTrip(t *testing.T) { in := &bucket.PolicyOK{ - ETag: `"bafy"`, Policy: &bucket.BucketPolicy{ Version: "2012-10-17", Statements: []bucket.Statement{ @@ -53,7 +52,7 @@ func TestPolicyOKRoundTrip(t *testing.T) { } func TestPolicyOKWithoutPolicy(t *testing.T) { - in := &bucket.PolicyOK{ETag: `"bafy"`} + in := &bucket.PolicyOK{} var cb bytes.Buffer require.NoError(t, in.MarshalCBOR(&cb)) var out bucket.PolicyOK diff --git a/commands/s3/bucket/types.go b/commands/s3/bucket/types.go index c17ed09..fdc351a 100644 --- a/commands/s3/bucket/types.go +++ b/commands/s3/bucket/types.go @@ -34,11 +34,10 @@ type PolicyArguments struct { Body []byte `cborgen:"body" dagjsongen:"body"` } -// PolicyOK is the successful result of `/s3/bucket/policy`. +// PolicyOK is the successful result of `/s3/bucket/policy`. The policy's +// entity tag is not carried: both sides compute it from the document with +// [ETag]. type PolicyOK struct { - // ETag is the tag of the bucket's policy after the operation. It is empty - // after a DeleteBucketPolicy. - ETag string `cborgen:"etag" dagjsongen:"etag"` // Policy is the stored policy document. It is set only for a // GetBucketPolicy. Policy *BucketPolicy `cborgen:"policy,omitempty" dagjsongen:"policy,omitempty"` From 491d3294a5312ef3b615d0df7656750f86b59e94 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Wed, 14 Oct 2026 11:55:13 +0200 Subject: [PATCH 8/9] refactor(s3): PolicyDocument with a StatementSet that is canonical on the wire The document type is PolicyDocument (the binding keeps the command's name, Policy). Its statements are a StatementSet: any order in memory, written deduplicated and ordered by statement CID in JSON, DAG-JSON and CBOR, so Normalize is gone and ETag is the CID of the document as encoded. The generators inline a slice field by kind, so the two-field document carries a hand-written codec that delegates to the set, laid out as the generated code was (a golden test holds the bytes). ETag and StatementCID share one CID helper. Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/cbor_gen.go | 171 +--------------------- commands/s3/bucket/etag.go | 63 ++------ commands/s3/bucket/etag_test.go | 101 +++++++++---- commands/s3/bucket/gen/main.go | 1 - commands/s3/bucket/json_gen.go | 186 +----------------------- commands/s3/bucket/policy_codec.go | 224 +++++++++++++++++++++++++++++ commands/s3/bucket/policy_json.go | 10 +- commands/s3/bucket/policy_test.go | 15 +- commands/s3/bucket/types.go | 19 ++- 9 files changed, 349 insertions(+), 441 deletions(-) create mode 100644 commands/s3/bucket/policy_codec.go diff --git a/commands/s3/bucket/cbor_gen.go b/commands/s3/bucket/cbor_gen.go index c32d0df..b532efe 100644 --- a/commands/s3/bucket/cbor_gen.go +++ b/commands/s3/bucket/cbor_gen.go @@ -453,7 +453,7 @@ func (t *PolicyOK) MarshalCBOR(w io.Writer) error { return err } - // t.Policy (bucket.BucketPolicy) (struct) + // t.Policy (bucket.PolicyDocument) (struct) if t.Policy != nil { if len("policy") > 8192 { @@ -515,7 +515,7 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { } switch string(nameBuf[:nameLen]) { - // t.Policy (bucket.BucketPolicy) (struct) + // t.Policy (bucket.PolicyDocument) (struct) case "policy": { @@ -528,7 +528,7 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { if err := cr.UnreadByte(); err != nil { return err } - t.Policy = new(BucketPolicy) + t.Policy = new(PolicyDocument) if err := t.Policy.UnmarshalCBOR(cr); err != nil { return xerrors.Errorf("unmarshaling t.Policy pointer: %w", err) } @@ -546,171 +546,6 @@ func (t *PolicyOK) UnmarshalCBOR(r io.Reader) (err error) { return nil } -func (t *BucketPolicy) MarshalCBOR(w io.Writer) error { - if t == nil { - _, err := w.Write(cbg.CborNull) - return err - } - - cw := cbg.NewCborWriter(w) - - if _, err := cw.Write([]byte{162}); err != nil { - return err - } - - // t.Version (string) (string) - if len("version") > 8192 { - return xerrors.Errorf("Value in field \"version\" was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("version"))); err != nil { - return err - } - if _, err := cw.WriteString(string("version")); err != nil { - return err - } - - if len(t.Version) > 8192 { - return xerrors.Errorf("Value in field t.Version was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len(t.Version))); err != nil { - return err - } - if _, err := cw.WriteString(string(t.Version)); err != nil { - return err - } - - // t.Statements ([]bucket.Statement) (slice) - if len("statement") > 8192 { - return xerrors.Errorf("Value in field \"statement\" was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajTextString, uint64(len("statement"))); err != nil { - return err - } - if _, err := cw.WriteString(string("statement")); err != nil { - return err - } - - if len(t.Statements) > 8192 { - return xerrors.Errorf("Slice value in field t.Statements was too long") - } - - if err := cw.WriteMajorTypeHeader(cbg.MajArray, uint64(len(t.Statements))); err != nil { - return err - } - for _, v := range t.Statements { - if err := v.MarshalCBOR(cw); err != nil { - return err - } - - } - return nil -} - -func (t *BucketPolicy) UnmarshalCBOR(r io.Reader) (err error) { - *t = BucketPolicy{} - - cr := cbg.NewCborReader(r) - - maj, extra, err := cr.ReadHeader() - if err != nil { - return err - } - defer func() { - if err == io.EOF { - err = io.ErrUnexpectedEOF - } - }() - - if maj != cbg.MajMap { - return fmt.Errorf("cbor input should be of type map") - } - - if extra > cbg.MaxLength { - return fmt.Errorf("BucketPolicy: map struct too large (%d)", extra) - } - - n := extra - - nameBuf := make([]byte, 9) - for i := uint64(0); i < n; i++ { - nameLen, ok, err := cbg.ReadFullStringIntoBuf(cr, nameBuf, 8192) - if err != nil { - return err - } - - if !ok { - // Field doesn't exist on this type, so ignore it - if err := cbg.ScanForLinks(cr, func(cid.Cid) {}); err != nil { - return err - } - continue - } - - switch string(nameBuf[:nameLen]) { - // t.Version (string) (string) - case "version": - - { - sval, err := cbg.ReadStringWithMax(cr, 8192) - if err != nil { - return err - } - - t.Version = string(sval) - } - // t.Statements ([]bucket.Statement) (slice) - case "statement": - - maj, extra, err = cr.ReadHeader() - if err != nil { - return err - } - - if extra > 8192 { - return fmt.Errorf("t.Statements: array too large (%d)", extra) - } - - if maj != cbg.MajArray { - return fmt.Errorf("expected cbor array") - } - - if extra > 0 { - t.Statements = make([]Statement, extra) - } - - for i := 0; i < int(extra); i++ { - { - var maj byte - var extra uint64 - var err error - _ = maj - _ = extra - _ = err - - { - - if err := t.Statements[i].UnmarshalCBOR(cr); err != nil { - return xerrors.Errorf("unmarshaling t.Statements[i]: %w", err) - } - - } - - } - } - - default: - // Field doesn't exist on this type, so ignore it - if err := cbg.ScanForLinks(r, func(cid.Cid) {}); err != nil { - return err - } - } - } - - return nil -} func (t *Statement) MarshalCBOR(w io.Writer) error { if t == nil { _, err := w.Write(cbg.CborNull) diff --git a/commands/s3/bucket/etag.go b/commands/s3/bucket/etag.go index dfdda62..5289fbd 100644 --- a/commands/s3/bucket/etag.go +++ b/commands/s3/bucket/etag.go @@ -7,64 +7,31 @@ package bucket import ( "bytes" - "slices" - "strings" + "io" "github.com/ipfs/go-cid" mh "github.com/multiformats/go-multihash" ) -// Normalize returns a copy of d in canonical form, its Version kept: the -// statements deduplicated and ordered by their CID (see [StatementCID]). The -// principal, action and resource sets need nothing, being sets. Two policies -// that differ only in statement order or in duplicate statements normalize -// to the same value. -func Normalize(d BucketPolicy) BucketPolicy { - type keyed struct { - key string - st Statement - } - keyedStatements := make([]keyed, 0, len(d.Statements)) - for _, st := range d.Statements { - keyedStatements = append(keyedStatements, keyed{key: StatementCID(st).String(), st: st}) - } - slices.SortFunc(keyedStatements, func(a, b keyed) int { return strings.Compare(a.key, b.key) }) - keyedStatements = slices.CompactFunc(keyedStatements, func(a, b keyed) bool { return a.key == b.key }) - statements := make([]Statement, len(keyedStatements)) - for i, k := range keyedStatements { - statements[i] = k.st - } - return BucketPolicy{Version: d.Version, Statements: statements} -} - -// ETag returns the entity tag of d: the CID of the DAG-JSON encoding of the -// [Normalize]d document (CIDv1, dag-json, sha2-256), its statements in CID -// order and each statement's sets as maps, in double quotes as HTTP carries -// it. Hilt stores it with the policy and checks If-Match against it; Ingot -// answers GetBucketPolicy and PutBucketPolicy with it. Callers treat it as -// opaque. -func ETag(d BucketPolicy) string { - n := Normalize(d) - var buf bytes.Buffer - if err := n.MarshalDagJSON(&buf); err != nil { - panic(err) // the sets are bounded by MaxSetSize, within the encoder's map limit - } - return `"` + dagJSONCID(buf.Bytes()).String() + `"` -} +// ETag returns the entity tag of d: the CID of its DAG-JSON encoding (CIDv1, +// dag-json, sha2-256), in double quotes as HTTP carries it. The encoding is +// canonical (see [StatementSet]), so two documents that differ only in +// statement order or duplicates share a tag. Hilt stores it with the policy +// and checks If-Match against it; Ingot answers GetBucketPolicy and +// PutBucketPolicy with it. Callers treat it as opaque. +func ETag(d PolicyDocument) string { return `"` + cidOf(&d).String() + `"` } // StatementCID returns the CID of st's DAG-JSON encoding: its sets as maps // with sorted keys, so nothing about how the caller built them reaches it. -func StatementCID(st Statement) cid.Cid { +func StatementCID(st Statement) cid.Cid { return cidOf(&st) } + +// cidOf returns the CIDv1 (dag-json, sha2-256) of m's DAG-JSON encoding. +func cidOf(m interface{ MarshalDagJSON(io.Writer) error }) cid.Cid { var buf bytes.Buffer - if err := st.MarshalDagJSON(&buf); err != nil { - panic(err) // see ETag + if err := m.MarshalDagJSON(&buf); err != nil { + panic(err) // the sets are bounded by MaxSetSize, within the encoders' limits } - return dagJSONCID(buf.Bytes()) -} - -// dagJSONCID returns the CIDv1 (dag-json, sha2-256) of raw. -func dagJSONCID(raw []byte) cid.Cid { - sum, err := mh.Sum(raw, mh.SHA2_256, -1) + sum, err := mh.Sum(buf.Bytes(), mh.SHA2_256, -1) if err != nil { panic(err) // sha2-256 with the default length cannot fail } diff --git a/commands/s3/bucket/etag_test.go b/commands/s3/bucket/etag_test.go index 25b1f02..5b0dd53 100644 --- a/commands/s3/bucket/etag_test.go +++ b/commands/s3/bucket/etag_test.go @@ -1,6 +1,9 @@ package bucket_test import ( + "bytes" + "encoding/hex" + "encoding/json" "slices" "strings" "testing" @@ -31,32 +34,83 @@ func deny(p bucket.Principal, actions ...string) bucket.Statement { return bucket.Statement{Effect: bucket.Deny, Principal: p, Actions: bucket.NewStringSet(actions...), Resources: both()} } -func doc(statements ...bucket.Statement) bucket.BucketPolicy { - return bucket.BucketPolicy{Version: bucket.Version2012, Statements: statements} +func doc(statements ...bucket.Statement) bucket.PolicyDocument { + return bucket.PolicyDocument{Version: bucket.Version2012, Statements: statements} } -func TestNormalize(t *testing.T) { - t.Run("orders statements by CID and drops duplicates", func(t *testing.T) { - a := doc(allow(only("alice"), "s3:GetObject"), deny(everyone, "s3:DeleteObject")) - b := doc(deny(everyone, "s3:DeleteObject"), allow(only("alice"), "s3:GetObject"), deny(everyone, "s3:DeleteObject")) - require.Equal(t, bucket.Normalize(a), bucket.Normalize(b)) - require.Len(t, bucket.Normalize(b).Statements, 2) - keys := []string{bucket.StatementCID(a.Statements[0]).String(), bucket.StatementCID(a.Statements[1]).String()} - require.True(t, slices.IsSorted(keys) || !slices.IsSorted(keys)) // either order is fine in a; the output is sorted: - n := bucket.Normalize(b) - require.True(t, bucket.StatementCID(n.Statements[0]).String() < bucket.StatementCID(n.Statements[1]).String()) +func TestStatementSet(t *testing.T) { + a := doc(allow(only("alice"), "s3:GetObject"), deny(everyone, "s3:DeleteObject")) + b := doc(deny(everyone, "s3:DeleteObject"), allow(only("alice"), "s3:GetObject"), deny(everyone, "s3:DeleteObject")) + + t.Run("every encoding orders statements by CID and drops duplicates", func(t *testing.T) { + ja, err := json.Marshal(a) + require.NoError(t, err) + jb, err := json.Marshal(b) + require.NoError(t, err) + require.Equal(t, string(ja), string(jb)) + require.Equal(t, 2, strings.Count(string(jb), `"Effect"`)) + + var da, db, ca, cb bytes.Buffer + require.NoError(t, a.MarshalDagJSON(&da)) + require.NoError(t, b.MarshalDagJSON(&db)) + require.Equal(t, da.String(), db.String()) + require.NoError(t, a.MarshalCBOR(&ca)) + require.NoError(t, b.MarshalCBOR(&cb)) + require.Equal(t, ca.Bytes(), cb.Bytes()) + + // The in-memory order is the caller's. + require.Equal(t, bucket.Deny, b.Statements[0].Effect) + require.Len(t, b.Statements, 3) }) - t.Run("keeps the version and does not alias the input", func(t *testing.T) { - d := doc(allow(only("alice"), "s3:GetObject")) - n := bucket.Normalize(d) - require.Equal(t, bucket.Version2012, n.Version) - n.Statements[0] = deny(everyone, "s3:GetObject") - require.Equal(t, bucket.Allow, d.Statements[0].Effect) + t.Run("decoding keeps the wire order, which is canonical", func(t *testing.T) { + var cb bytes.Buffer + require.NoError(t, b.MarshalCBOR(&cb)) + var back bucket.PolicyDocument + require.NoError(t, back.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Len(t, back.Statements, 2) + require.True(t, bucket.StatementCID(back.Statements[0]).String() < bucket.StatementCID(back.Statements[1]).String()) + require.Equal(t, a.Version, back.Version) + + var jb bytes.Buffer + require.NoError(t, b.MarshalDagJSON(&jb)) + var backJSON bucket.PolicyDocument + require.NoError(t, backJSON.UnmarshalDagJSON(bytes.NewReader(jb.Bytes()))) + require.Equal(t, back, backJSON) }) - t.Run("nil and empty statement lists normalize the same", func(t *testing.T) { - require.Equal(t, bucket.Normalize(bucket.BucketPolicy{}), bucket.Normalize(bucket.BucketPolicy{Statements: []bucket.Statement{}})) + t.Run("nil and empty encode the same", func(t *testing.T) { + var n1, n2 bytes.Buffer + require.NoError(t, (&bucket.PolicyDocument{}).MarshalDagJSON(&n1)) + require.NoError(t, (&bucket.PolicyDocument{Statements: bucket.StatementSet{}}).MarshalDagJSON(&n2)) + require.Equal(t, `{"statement":[],"version":""}`, n1.String()) + require.Equal(t, n1.String(), n2.String()) + raw, err := json.Marshal(bucket.PolicyDocument{}) + require.NoError(t, err) + require.Equal(t, `{"Version":"","Statement":[]}`, string(raw)) + }) + + t.Run("CBOR and DAG-JSON match the generated layout", func(t *testing.T) { + // Captured from the generated encoders before the codec was hand-written: + // CBOR keys shortest first (version, statement), DAG-JSON keys sorted. + const cborHex = "a26776657273696f6e6a323031322d31302d31376973746174656d656e7481a466616374696f6ea16c73333a4765744f626a656374a06665666665637465416c6c6f77687265736f75726365a27361726e3a6177733a73333a3a3a70686f746f73a07561726e3a6177733a73333a3a3a70686f746f732f2aa0697072696e636970616ca165616c696365a0" + const okCBORHex = "a166706f6c696379" + cborHex + d := doc(allow(only("alice"), "s3:GetObject")) + var cb bytes.Buffer + require.NoError(t, d.MarshalCBOR(&cb)) + require.Equal(t, cborHex, hex.EncodeToString(cb.Bytes())) + var back bucket.PolicyDocument + require.NoError(t, back.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) + require.Equal(t, d, back) + + ok := bucket.PolicyOK{Policy: &d} + cb.Reset() + require.NoError(t, ok.MarshalCBOR(&cb)) + require.Equal(t, okCBORHex, hex.EncodeToString(cb.Bytes())) + + var jb bytes.Buffer + require.NoError(t, d.MarshalDagJSON(&jb)) + require.Equal(t, `{"statement":[{"action":{"s3:GetObject":{}},"effect":"Allow","principal":{"alice":{}},"resource":{"arn:aws:s3:::photos":{},"arn:aws:s3:::photos/*":{}}}],"version":"2012-10-17"}`, jb.String()) }) } @@ -71,7 +125,6 @@ func TestETag(t *testing.T) { require.EqualValues(t, cid.DagJSON, c.Type()) statement := `{"action":{"s3:GetObject":{}},"effect":"Allow","principal":{"alice":{}},"resource":{"arn:aws:s3:::photos":{},"arn:aws:s3:::photos/*":{}}}` require.Equal(t, dagJSONCID(t, dagJSONPolicy(t, "2012-10-17", statement)), c) - require.Equal(t, bucket.ETag(bucket.Normalize(d)), tag) }) t.Run("encodes a sid, a wildcard principal and several statements", func(t *testing.T) { @@ -119,10 +172,10 @@ func TestETag(t *testing.T) { // from hand-assembled DAG-JSON bytes: // {"statement":[],"version":""} and // {"statement":[],"version":"2012-10-17"}. - require.Equal(t, `"baguqeerapfhqlt74verbk25awofci4fsi4dewjbihdbnexfwphaywy4srd2q"`, bucket.ETag(bucket.BucketPolicy{})) + require.Equal(t, `"baguqeerapfhqlt74verbk25awofci4fsi4dewjbihdbnexfwphaywy4srd2q"`, bucket.ETag(bucket.PolicyDocument{})) require.Equal(t, `"baguqeeramb7bdvrj6whijwstk5ktmw3z3wrwg64cf4xfj6qb2xxgsxsj7emq"`, bucket.ETag(d)) - require.Equal(t, bucket.ETag(bucket.BucketPolicy{}), bucket.ETag(bucket.BucketPolicy{Statements: []bucket.Statement{}})) - require.Equal(t, `"`+dagJSONCID(t, `{"statement":[],"version":""}`).String()+`"`, bucket.ETag(bucket.BucketPolicy{})) + require.Equal(t, bucket.ETag(bucket.PolicyDocument{}), bucket.ETag(bucket.PolicyDocument{Statements: []bucket.Statement{}})) + require.Equal(t, `"`+dagJSONCID(t, `{"statement":[],"version":""}`).String()+`"`, bucket.ETag(bucket.PolicyDocument{})) }) } diff --git a/commands/s3/bucket/gen/main.go b/commands/s3/bucket/gen/main.go index 3075dc9..a4d70d4 100644 --- a/commands/s3/bucket/gen/main.go +++ b/commands/s3/bucket/gen/main.go @@ -29,7 +29,6 @@ func main() { bucket.ListArguments{}, bucket.PolicyArguments{}, bucket.PolicyOK{}, - bucket.BucketPolicy{}, bucket.Statement{}, bucket.InfoArguments{}, bucket.InfoOK{}, diff --git a/commands/s3/bucket/json_gen.go b/commands/s3/bucket/json_gen.go index 6d84c1e..76c4965 100644 --- a/commands/s3/bucket/json_gen.go +++ b/commands/s3/bucket/json_gen.go @@ -437,7 +437,7 @@ func (t *PolicyOK) MarshalDagJSON(w io.Writer) error { return err } - // t.Policy (bucket.BucketPolicy) (struct) + // t.Policy (bucket.PolicyDocument) (struct) if t.Policy != nil { if len("policy") > 8192 { return fmt.Errorf("string in field \"policy\" was too long") @@ -491,7 +491,7 @@ func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { } switch name { - // t.Policy (bucket.BucketPolicy) (struct) + // t.Policy (bucket.PolicyDocument) (struct) case "policy": { @@ -504,7 +504,7 @@ func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { return fmt.Errorf("reading null for field t.Policy: %w", err) } } else { - t.Policy = new(BucketPolicy) + t.Policy = new(PolicyDocument) if err := t.Policy.UnmarshalDagJSON(jr); err != nil { return fmt.Errorf("unmarshaling t.Policy pointer: %w", err) } @@ -533,186 +533,6 @@ func (t *PolicyOK) UnmarshalDagJSON(r io.Reader) (err error) { return nil } -func (t *BucketPolicy) MarshalDagJSON(w io.Writer) error { - jw := jsg.NewDagJsonWriter(w) - if t == nil { - err := jw.WriteNull() - return err - } - if err := jw.WriteObjectOpen(); err != nil { - return err - } - written := false - - // t.Statements ([]bucket.Statement) (slice) - if len("statement") > 8192 { - return fmt.Errorf("string in field \"statement\" was too long") - } - if err := jw.WriteString(string("statement")); err != nil { - return fmt.Errorf("writing string for field \"statement\": %w", err) - } - if err := jw.WriteObjectColon(); err != nil { - return err - } - if len(t.Statements) > 8192 { - return fmt.Errorf("slice value in field t.Statements was too long") - } - - if err := jw.WriteArrayOpen(); err != nil { - return fmt.Errorf("writing array open for field t.Statements: %w", err) - } - for i, v := range t.Statements { - if i > 0 { - if err := jw.WriteComma(); err != nil { - return fmt.Errorf("writing comma for field t.Statements: %w", err) - } - } - if err := v.MarshalDagJSON(jw); err != nil { - return fmt.Errorf("marshaling field v: %w", err) - } - } - if err := jw.WriteArrayClose(); err != nil { - return fmt.Errorf("writing array close for field t.Statements: %w", err) - } - - written = true - if written { - if err := jw.WriteComma(); err != nil { - return err - } - } - - // t.Version (string) (string) - if len("version") > 8192 { - return fmt.Errorf("string in field \"version\" was too long") - } - if err := jw.WriteString(string("version")); err != nil { - return fmt.Errorf("writing string for field \"version\": %w", err) - } - if err := jw.WriteObjectColon(); err != nil { - return err - } - if len(t.Version) > 8192 { - return fmt.Errorf("string in field t.Version was too long") - } - if err := jw.WriteString(string(t.Version)); err != nil { - return fmt.Errorf("writing string for field t.Version: %w", err) - } - if err := jw.WriteObjectClose(); err != nil { - return err - } - return nil -} -func (t *BucketPolicy) UnmarshalDagJSON(r io.Reader) (err error) { - *t = BucketPolicy{} - - jr := jsg.NewDagJsonReader(r) - defer func() { - if err == io.EOF { - err = io.ErrUnexpectedEOF - } - }() - if err := jr.ReadObjectOpen(); err != nil { - return fmt.Errorf("reading object open for BucketPolicy: %w", err) - } - close, err := jr.PeekObjectClose() - if err != nil { - return fmt.Errorf("peeking object close for BucketPolicy: %w", err) - } - if close { - if err := jr.ReadObjectClose(); err != nil { - return fmt.Errorf("reading object close for BucketPolicy: %w", err) - } - } else { - for i := uint64(0); i < 8192; i++ { - name, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field BucketPolicy: string too large") - } - return fmt.Errorf("reading string for field BucketPolicy: %w", err) - } - if err := jr.ReadObjectColon(); err != nil { - return fmt.Errorf("reading object colon for field BucketPolicy: %w", err) - } - switch name { - - // t.Statements ([]bucket.Statement) (slice) - case "statement": - { - - if err := jr.ReadArrayOpen(); err != nil { - return fmt.Errorf("reading array open for field t.Statements: %w", err) - } - - close, err := jr.PeekArrayClose() - if err != nil { - return fmt.Errorf("peeking array close for field t.Statements: %w", err) - } - if close { - if err := jr.ReadArrayClose(); err != nil { - return fmt.Errorf("reading array close for field t.Statements: %w", err) - } - - } else { - for i := 0; i < 8192; i++ { - item := make([]Statement, 1) - - if err := item[0].UnmarshalDagJSON(jr); err != nil { - return fmt.Errorf("unmarshaling item[0]: %w", err) - } - - t.Statements = append(t.Statements, item[0]) - - close, err := jr.ReadArrayCloseOrComma() - if err != nil { - return fmt.Errorf("reading array close or comma for field t.Statements: %w", err) - } - if close { - break - } - if i == 8192-1 { - return fmt.Errorf("reading array for field t.Statements: slice too large") - } - } - } - - } - - // t.Version (string) (string) - case "version": - { - sval, err := jr.ReadString(8192) - if err != nil { - if errors.Is(err, jsg.ErrLimitExceeded) { - return fmt.Errorf("reading string for field t.Version: string too long") - } - return fmt.Errorf("reading string for field t.Version: %w", err) - } - t.Version = string(sval) - } - default: - // Field doesn't exist on this type, so ignore it - if err := jr.DiscardType(); err != nil { - return fmt.Errorf("ignoring field %s for BucketPolicy: %w", name, err) - } - } - - close, err := jr.ReadObjectCloseOrComma() - if err != nil { - return fmt.Errorf("reading object close or comma for field BucketPolicy: %w", err) - } - if close { - break - } - if i == 8192-1 { - return fmt.Errorf("map too large for BucketPolicy") - } - } - } - - return nil -} func (t *Statement) MarshalDagJSON(w io.Writer) error { jw := jsg.NewDagJsonWriter(w) if t == nil { diff --git a/commands/s3/bucket/policy_codec.go b/commands/s3/bucket/policy_codec.go new file mode 100644 index 0000000..1d05cc4 --- /dev/null +++ b/commands/s3/bucket/policy_codec.go @@ -0,0 +1,224 @@ +//go:build !codegen + +// The codecs below call generated encoders, which the generator builds the +// package without. + +package bucket + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "slices" + "strings" + + jsg "github.com/alanshaw/dag-json-gen" + "github.com/fil-forge/libforge/commands/internal/codec" + cbg "github.com/whyrusleeping/cbor-gen" +) + +// canonical returns the statements deduplicated and ordered by +// [StatementCID]: the order every encoding writes. The result is never nil. +func (s StatementSet) canonical() []Statement { + type keyed struct { + key string + st Statement + } + keyed_ := make([]keyed, 0, len(s)) + for _, st := range s { + keyed_ = append(keyed_, keyed{key: StatementCID(st).String(), st: st}) + } + slices.SortFunc(keyed_, func(a, b keyed) int { return strings.Compare(a.key, b.key) }) + keyed_ = slices.CompactFunc(keyed_, func(a, b keyed) bool { return a.key == b.key }) + out := make([]Statement, len(keyed_)) + for i, k := range keyed_ { + out[i] = k.st + } + return out +} + +// MarshalJSON encodes the set as a list in canonical order; nil encodes as +// an empty list. +func (s StatementSet) MarshalJSON() ([]byte, error) { return json.Marshal(s.canonical()) } + +// UnmarshalJSON decodes a list of statements, keeping the order given. +func (s *StatementSet) UnmarshalJSON(data []byte) error { + var list []Statement + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(&list); err != nil { + return err + } + *s = list + return nil +} + +// MarshalCBOR writes the set as an array in canonical order. +func (s StatementSet) MarshalCBOR(w io.Writer) error { + cw := cbg.NewCborWriter(w) + c := s.canonical() + if err := codec.WriteCborArrayHeader(cw, len(c)); err != nil { + return err + } + for i := range c { + if err := c[i].MarshalCBOR(cw); err != nil { + return err + } + } + return nil +} + +// UnmarshalCBOR reads an array of statements, at most [MaxSetSize]. +func (s *StatementSet) UnmarshalCBOR(r io.Reader) error { + cr := cbg.NewCborReader(r) + n, err := codec.ReadCborArrayHeader(cr) + if err != nil { + return err + } + if n > MaxSetSize { + return fmt.Errorf("statement set holds %d statements, at most %d are allowed", n, MaxSetSize) + } + out := make(StatementSet, n) + for i := range out { + if err := out[i].UnmarshalCBOR(cr); err != nil { + return err + } + } + *s = out + return nil +} + +// MarshalDagJSON writes the set as an array in canonical order. +func (s StatementSet) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if err := jw.WriteArrayOpen(); err != nil { + return err + } + for i, st := range s.canonical() { + if i > 0 { + if err := jw.WriteComma(); err != nil { + return err + } + } + if err := st.MarshalDagJSON(jw); err != nil { + return err + } + } + return jw.WriteArrayClose() +} + +// UnmarshalDagJSON reads an array of statements, at most [MaxSetSize]. +func (s *StatementSet) UnmarshalDagJSON(r io.Reader) error { + jr := jsg.NewDagJsonReader(r) + list, err := codec.ReadJSONArray(jr, func() (Statement, error) { + var st Statement + err := st.UnmarshalDagJSON(jr) + return st, err + }) + if err != nil { + return err + } + if len(list) > MaxSetSize { + return fmt.Errorf("statement set holds %d statements, at most %d are allowed", len(list), MaxSetSize) + } + *s = list + return nil +} + +// The document's own codecs are hand-written because the generators inline a +// slice field by its kind and would bypass StatementSet's canonical order. +// They write the layout the generators would: CBOR keys shortest first, +// DAG-JSON keys sorted. + +// MarshalCBOR writes d as a map of version and statement. +func (d *PolicyDocument) MarshalCBOR(w io.Writer) error { + cw := cbg.NewCborWriter(w) + if d == nil { + _, err := cw.Write(cbg.CborNull) + return err + } + if err := codec.WriteCborMapHeader(cw, 2); err != nil { + return err + } + if err := codec.WriteCborString(cw, "version"); err != nil { + return err + } + if err := codec.WriteCborString(cw, d.Version); err != nil { + return err + } + if err := codec.WriteCborString(cw, "statement"); err != nil { + return err + } + return d.Statements.MarshalCBOR(cw) +} + +// UnmarshalCBOR reads a map of version and statement. +func (d *PolicyDocument) UnmarshalCBOR(r io.Reader) error { + *d = PolicyDocument{} + cr := cbg.NewCborReader(r) + n, err := codec.ReadCborMapHeader(cr) + if err != nil { + return err + } + for i := uint64(0); i < n; i++ { + key, err := codec.ReadCborString(cr) + if err != nil { + return err + } + switch key { + case "version": + d.Version, err = codec.ReadCborString(cr) + case "statement": + err = d.Statements.UnmarshalCBOR(cr) + default: + return fmt.Errorf("unknown field %q in a policy document", key) + } + if err != nil { + return err + } + } + return nil +} + +// MarshalDagJSON writes d as an object of statement and version. +func (d *PolicyDocument) MarshalDagJSON(w io.Writer) error { + jw := jsg.NewDagJsonWriter(w) + if d == nil { + return jw.WriteNull() + } + if err := jw.WriteObjectOpen(); err != nil { + return err + } + if err := codec.WriteJSONKey(jw, "statement", false); err != nil { + return err + } + if err := d.Statements.MarshalDagJSON(jw); err != nil { + return err + } + if err := codec.WriteJSONKey(jw, "version", true); err != nil { + return err + } + if err := jw.WriteString(d.Version); err != nil { + return err + } + return jw.WriteObjectClose() +} + +// UnmarshalDagJSON reads an object of statement and version. +func (d *PolicyDocument) UnmarshalDagJSON(r io.Reader) error { + *d = PolicyDocument{} + jr := jsg.NewDagJsonReader(r) + return codec.ReadJSONObject(jr, func(name string) error { + switch name { + case "version": + v, err := jr.ReadString(codec.MaxString) + d.Version = v + return err + case "statement": + return d.Statements.UnmarshalDagJSON(jr) + default: + return fmt.Errorf("unknown field %q in a policy document", name) + } + }) +} diff --git a/commands/s3/bucket/policy_json.go b/commands/s3/bucket/policy_json.go index fc94d57..ad6267f 100644 --- a/commands/s3/bucket/policy_json.go +++ b/commands/s3/bucket/policy_json.go @@ -278,11 +278,11 @@ func (p *Principal) UnmarshalJSON(data []byte) error { // case, a malformed principal, or a document [WellFormed] rejects is an error // wrapping [ErrInvalidPolicy]. Action and Resource may each be a string or a // list; the lists are sets, so order and duplicates do not survive. -func (p *BucketPolicy) UnmarshalJSON(data []byte) error { +func (p *PolicyDocument) UnmarshalJSON(data []byte) error { if err := strictKeys(data); err != nil { return err } - type plain BucketPolicy // the struct without this method + type plain PolicyDocument // the struct without this method var d plain dec := json.NewDecoder(bytes.NewReader(data)) dec.DisallowUnknownFields() @@ -292,10 +292,10 @@ func (p *BucketPolicy) UnmarshalJSON(data []byte) error { } return fmt.Errorf("decoding policy: %v: %w", err, ErrInvalidPolicy) } - if err := WellFormed(BucketPolicy(d)); err != nil { + if err := WellFormed(PolicyDocument(d)); err != nil { return err } - *p = BucketPolicy(d) + *p = PolicyDocument(d) return nil } @@ -345,7 +345,7 @@ func strictKeys(data []byte) error { // names, covering what its actions act on: an object action needs the objects // ARN in its statement, a bucket action the bucket ARN, and "s3:*" both. It // returns an error wrapping [ErrInvalidPolicy] otherwise. -func WellFormed(d BucketPolicy) error { +func WellFormed(d PolicyDocument) error { if d.Version != Version2012 && d.Version != Version2008 { return fmt.Errorf("policy Version %q must be %q or %q: %w", d.Version, Version2012, Version2008, ErrInvalidPolicy) } diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index b448399..75240bb 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -12,7 +12,7 @@ import ( func TestPolicyOKRoundTrip(t *testing.T) { in := &bucket.PolicyOK{ - Policy: &bucket.BucketPolicy{ + Policy: &bucket.PolicyDocument{ Version: "2012-10-17", Statements: []bucket.Statement{ { @@ -32,17 +32,20 @@ func TestPolicyOKRoundTrip(t *testing.T) { }, } + // The wire carries the statements in canonical order, so a round trip + // keeps the set, not the order the fixture lists it in. var cb bytes.Buffer require.NoError(t, in.MarshalCBOR(&cb)) var out bucket.PolicyOK require.NoError(t, out.UnmarshalCBOR(bytes.NewReader(cb.Bytes()))) - require.Equal(t, *in, out) + require.Equal(t, in.Policy.Version, out.Policy.Version) + require.ElementsMatch(t, in.Policy.Statements, out.Policy.Statements) var jb bytes.Buffer require.NoError(t, in.MarshalDagJSON(&jb)) var outJSON bucket.PolicyOK require.NoError(t, outJSON.UnmarshalDagJSON(bytes.NewReader(jb.Bytes()))) - require.Equal(t, *in, outJSON) + require.Equal(t, out, outJSON) aws, err := json.Marshal(out.Policy) require.NoError(t, err) @@ -141,12 +144,12 @@ const res = `"Resource": ["arn:aws:s3:::photos", "arn:aws:s3:::photos/*"]` func TestBucketPolicyUnmarshalJSON(t *testing.T) { t.Run("decodes the documented shape, keeping its order", func(t *testing.T) { - var d bucket.BucketPolicy + var d bucket.PolicyDocument require.NoError(t, json.Unmarshal([]byte(policyJSON( `{"Sid": "owners", "Effect": "Allow", "Principal": {"AWS": ["arn:aws:iam:::user/bob", "arn:aws:iam:::user/alice"]}, "Action": ["s3:ListBucket", "s3:GetObject"], `+res+`}`, `{"Effect": "Deny", "Principal": "*", "Action": "s3:PutObjectRetention", "Resource": "arn:aws:s3:::photos/*"}`, )), &d)) - require.Equal(t, bucket.BucketPolicy{Version: bucket.Version2012, Statements: []bucket.Statement{ + require.Equal(t, bucket.PolicyDocument{Version: bucket.Version2012, Statements: []bucket.Statement{ {Sid: "owners", Effect: bucket.Allow, Principal: bucket.Only("bob", "alice"), Actions: bucket.NewStringSet("s3:ListBucket", "s3:GetObject"), Resources: bucket.NewStringSet(bucket.BucketARN("photos"), bucket.ObjectsARN("photos"))}, {Effect: bucket.Deny, Principal: bucket.Everyone(), Actions: bucket.NewStringSet("s3:PutObjectRetention"), Resources: bucket.NewStringSet(bucket.ObjectsARN("photos"))}, }}, d) @@ -215,7 +218,7 @@ func TestBucketPolicyUnmarshalJSON(t *testing.T) { } for _, tt := range rejects { t.Run("rejects "+tt.name, func(t *testing.T) { - var d bucket.BucketPolicy + var d bucket.PolicyDocument err := json.Unmarshal([]byte(tt.body), &d) require.ErrorIs(t, err, bucket.ErrInvalidPolicy) require.ErrorContains(t, err, tt.want) diff --git a/commands/s3/bucket/types.go b/commands/s3/bucket/types.go index fdc351a..dee8c25 100644 --- a/commands/s3/bucket/types.go +++ b/commands/s3/bucket/types.go @@ -40,16 +40,23 @@ type PolicyArguments struct { type PolicyOK struct { // Policy is the stored policy document. It is set only for a // GetBucketPolicy. - Policy *BucketPolicy `cborgen:"policy,omitempty" dagjsongen:"policy,omitempty"` + Policy *PolicyDocument `cborgen:"policy,omitempty" dagjsongen:"policy,omitempty"` } -// BucketPolicy is a bucket policy document. Its JSON encoding is the AWS -// bucket policy document. -type BucketPolicy struct { - Version string `cborgen:"version" dagjsongen:"version" json:"Version"` - Statements []Statement `cborgen:"statement" dagjsongen:"statement" json:"Statement"` +// PolicyDocument is a bucket policy document. Its JSON encoding is the AWS +// bucket policy document; its CBOR and DAG-JSON encodings are hand-written +// (policy_codec.go) so that the statements go out in canonical order. +type PolicyDocument struct { + Version string `json:"Version"` + Statements StatementSet `json:"Statement"` } +// StatementSet is a policy's statements: any order in memory, canonical on +// the wire. Every encoding (JSON, DAG-JSON, CBOR) writes them deduplicated +// and ordered by [StatementCID], so two documents that differ only in +// statement order or duplicates encode the same. +type StatementSet []Statement + // Effect is whether a [Statement] grants or withholds its actions. type Effect string From 222cd95eb466dec3761921ddd3fa0e102ff4a881 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Wed, 14 Oct 2026 11:55:45 +0200 Subject: [PATCH 9/9] test(s3): expect the receipt's JSON statements in CID order Co-Authored-By: Claude Fable 5.1 --- commands/s3/bucket/policy_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/commands/s3/bucket/policy_test.go b/commands/s3/bucket/policy_test.go index 75240bb..8a71a61 100644 --- a/commands/s3/bucket/policy_test.go +++ b/commands/s3/bucket/policy_test.go @@ -49,9 +49,10 @@ func TestPolicyOKRoundTrip(t *testing.T) { aws, err := json.Marshal(out.Policy) require.NoError(t, err) + // Statements come out in CID order: the deny's sorts before the readers'. require.JSONEq(t, `{"Version":"2012-10-17","Statement":[`+ - `{"Sid":"readers","Effect":"Allow","Principal":{"AWS":["arn:aws:iam:::user/alice","arn:aws:iam:::user/bob"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::b/*"]},`+ - `{"Effect":"Deny","Principal":"*","Action":["s3:DeleteObject"],"Resource":["arn:aws:s3:::b/*"]}]}`, string(aws)) + `{"Effect":"Deny","Principal":"*","Action":["s3:DeleteObject"],"Resource":["arn:aws:s3:::b/*"]},`+ + `{"Sid":"readers","Effect":"Allow","Principal":{"AWS":["arn:aws:iam:::user/alice","arn:aws:iam:::user/bob"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::b/*"]}]}`, string(aws)) } func TestPolicyOKWithoutPolicy(t *testing.T) {