carlos / platform Public

The principal report: the second review's six P2s (repeated members, default-version agreement, trust-policy grammar, access-path reaches, and the two proposals stated with what they lack)

Astra's second review of 199a9736: reject, six P2s. Answered here:

- A JSON member given twice anywhere in a document is an error (the
  standard decoder kept the last; the first might be the one that
  reaches).
- A managed policy's own default-version id must name the version marked
  default, or the policy is not read.
- Every trust-policy statement is read in full, Deny included: the
  element set, the action element, the principal element's shape and
  kinds (AWS, Service, Federated, CanonicalUser), and NotPrincipal's
  shape; a condition must be a map of operators, a sid and an id strings.
- A grant on an access point, a multi-region access point or an Object
  Lambda access point is a possible reach of every bucket, reported and
  marked so, never dropped.
- The note states what proposals A and B lack (an enforced binding of
  each store identity to its bucket; the other access paths; a protected
  control verified apart from this report; provenance that does not rest
  on unpredictability), and that the report cannot verify any rule.

Twelve more mutants in a tree copy; all die. Nothing was run against
AWS; nothing is wired.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude 3 weeks ago 0e1bc298c9cd67122cdd7389d7f7718048de2283 parent 199a973

3 files changed, +378 −107

docs/superpowers/plans/2026-10-02-aws-principal-inventory.md +11 −11
internal/objstore/principals.go +264 −57
internal/objstore/principals_test.go +103 −39

CI passed — run details

diff --git a/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md b/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md

index b2565f663..43dcda596 100644

--- a/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md

+++ b/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md

@@ -1,6 +1,6 @@

# The AWS principal report, and what a completeness claim needs

-Date: 2026-10-02. Revised the same day after its first review (Astra: reject; three P1s, two P2s, one P3; the Opus review has not run). Status: a report for a person, built on `feat/carloku-principal-inventory-20261002`. **It is not wired, and it is no longer offered as the proof a bucket's ledger needs.** Nothing was run against AWS.

+Date: 2026-10-02. Revised the same day after its first review (Astra: reject; three P1s, two P2s, one P3) and on 2026-10-03 after its second (Astra: reject; six P2s about the report's own completeness and the two proposals; the Opus review has not run). Status: a report for a person, built on `feat/carloku-principal-inventory-20261002`. **It is not wired, and it is no longer offered as the proof a bucket's ledger needs.** Nothing was run against AWS.

## Why this exists

@@ -16,28 +16,28 @@ So the branch now does less, and says so.

## What is built

-`internal/objstore/principals.go`: `PrincipalReport.BucketPrincipals(name)`. One listing of the account (`iam:GetAccountAuthorizationDetails`, every page), every identity policy decoded and evaluated against one bucket name, and a report: which users, roles and groups reach the name, through which policies, and for each role, who its trust policy lets assume it. Principals the deployment names as its own are set apart in the report. That is a label for the reader and exempts nothing.

+`internal/objstore/principals.go`: `PrincipalReport.BucketPrincipals(name)`. One listing of the account (`iam:GetAccountAuthorizationDetails`, every page), every identity policy decoded and evaluated against one bucket name, and a report: which users, roles and groups reach the name, directly (the bucket's ARN or an object under it) or possibly (a grant on an access point, a multi-region access point or an Object Lambda access point, which names no bucket and can front any bucket's objects; marked so, never dropped), through which policies, and for each role, who its trust policy lets assume it. Principals the deployment names as its own are set apart in the report. That is a label for the reader and exempts nothing.

-- It is **not** a `GranteeProber`, and a test says so. The grant walk on AWS claims nothing, as before.

+- It is **not** a `GranteeProber`, and a test says so. The grant walk on AWS claims nothing, as before. It is a report to support a person's investigation; it cannot verify that any rule is in force in the account, and identical reports can come from an account with and without one.

- Within the documents it is given it leans toward listing: a Deny, a condition or a boundary is not evaluated; `NotAction`, `NotResource` and policy variables are taken to reach.

-- What it cannot read is an error, never a shorter list: an undecodable document; a statement, element or version outside the identity-policy grammar (a null or empty `Statement`, an empty or null value, a `Principal`, an unknown element); the same principal, managed policy or inline policy name returned twice; an attached policy the listing did not return; a truncated or endless listing.

-- It does not read: the root user; a bucket's own policy, ACL or access points; grants through other services' resources (access points, Object Lambda, Batch Operations, replication jobs); who inside the account may assume a role (`sts:AssumeRole` in their own policies); anything in another account.

+- What it cannot read is an error, never a shorter list: an undecodable document; a JSON member given twice anywhere in a document; a statement, element or version outside the grammar it knows, in an identity policy or a trust policy, Deny statements included (a null or empty `Statement`, an empty or null value, a `Principal` in an identity policy, an unknown element, a condition or a sid of the wrong shape, an unknown principal kind in a trust policy); the same principal, managed policy or inline policy name returned twice; a managed policy whose own default-version id and the version marked default disagree; an attached policy the listing did not return; a truncated or endless listing.

+- It does not read: the root user; a bucket's own policy, ACL or its access points' own policies; what Batch Operations and replication jobs may do under their roles; who inside the account may assume a role (`sts:AssumeRole` in their own policies); anything in another account; organization policies and permissions boundaries (which only narrow).

Use: a person preparing the attestation of the four existing stores, or checking a deployment, can see who reaches a store name today. It needs `iam:GetAccountAuthorizationDetails`, which the console's role does not have and this branch does not add; it can be run with an operator's own credentials.

-Tests: `go test ./internal/objstore -run 'TestPolicyPatterns|TestInventory|TestPlatformPrincipals|TestNothingOnAWS'`, against a stub of the listing. No test reads a real account, and the stub is built from SDK structs, not from IAM's wire format.

+Tests: `go test ./internal/objstore -run 'TestPolicyPatterns|TestInventory|TestPlatformPrincipals|TestNothingOnAWS|TestAnAccessPoint'`, against a stub of the listing. No test reads a real account, and the stub is built from SDK structs, not from IAM's wire format.

-Mutation checks, in a tree copy, forty. Thirty-nine die. One is equivalent: dropping the check that a default version has a document changes nothing, because an absent document then fails to decode. One mutant did not compile as first written and was rerun. Output: `inventory-mutants-round1.txt` in the run directory's logs.

+Mutation checks, in a tree copy: forty after the first review (thirty-nine die; one is equivalent, dropping the check that a default version has a document, because an absent document then fails to decode), and twelve after the second (repeated members not checked, and not inside a statement; the default-version id not compared; an access point grant, and an Object Lambda grant, nothing; a trust Deny not read in full; an unknown principal kind a label; a condition, and a sid, of any shape; a trust statement with no action; a possible reach read as direct; a star spanning fields no access path), all twelve die. Output: `inventory-mutants-round1.txt`, `inventory-mutants-round2.txt` in the run directory's logs.

## What a completeness claim needs instead (not built; needs the owner)

-Looking cannot establish it. A rule the account enforces can. Two parts, each a design question:

+Looking cannot establish it. A rule the account enforces can. Two parts, each a design question, each stated here with what its review found missing in it:

-**A. Nobody else can be granted a store bucket.** Proposed for review: an organization policy (or, failing that, a permissions boundary required on every principal the platform creates plus an account rule) that denies S3 actions on the store bucket prefix to every principal except the store users under the store path and a short list named by ARN. With that in force, "who can reach this bucket" is the store user and the named list by construction, and the report above becomes the check that the rule is in place, not the proof. Who is on the named list, and who can assume each role on it, is decision 14.

+**A. Nobody else can reach a store bucket.** Proposed direction: an organization policy (or, failing that, a permissions boundary required on every principal the platform creates plus an account rule) that denies S3 actions on the store bucket prefix to every principal except each bucket's own store user and a short list named by ARN. What that needs, beyond the sentence: a binding between each store identity and its one bucket, enforced (a store user under the path with a policy for another store's bucket must be impossible, not merely absent); coverage of the paths an identity policy does not show (resource policies and grants to other accounts, service-linked roles, access points); the enforcement configuration itself protected from the principals it constrains; and a way to verify the control is in force that is separate from this report, which cannot see it. Who is on the named list, and who can assume each role on it, is decision 14.

-**B. The bucket is the one this grant made.** Proposed for review: the grant mints a bucket name with a random part recorded in its grant block before any provider call, so no other actor can know the name to create it first; and the walk refuses a bucket that already exists under that name for any reason, including AWS's "already owned by you".

+**B. The bucket is the one this grant made.** Proposed direction: the grant mints a bucket name with a random part recorded in its grant block before any provider call, and refuses a bucket that already exists under that name. What that needs, beyond the sentence: randomness only makes an accidental collision and a guess unlikely; an administrator who can read the grant block, or the store user's policy written before the bucket, can create the name first, so creation provenance needs an enforceable restriction on who may create buckets under the prefix, not unpredictability; an explicit treatment of the provider answering an already-owned bucket's create as success (the us-east-1 case the provisioner already notes) and of a retry after the platform's own create; and crash recovery that tells the platform's own earlier create from someone else's.

-Neither is designed in detail, and both change infrastructure or the grant walk, which need their own review.

+Neither is designed, and both change infrastructure or the grant walk, which need their own review. Until both, no bucket's ledger on AWS claims completeness and store transfer holds.

## Decision for Paul (item 14, reworded)

diff --git a/internal/objstore/principals.go b/internal/objstore/principals.go

index b41ce9ef1..118fc97cf 100644

--- a/internal/objstore/principals.go

+++ b/internal/objstore/principals.go

@@ -5,6 +5,7 @@ import (

"encoding/json"

"errors"

"fmt"

+ "io"

"net/url"

"sort"

"strings"

@@ -59,17 +60,23 @@ import (

// expand to anything.

// - A group whose policy reaches the bucket is listed, with or without

// members, and so is each of its members.

+// - A grant on an access point, a multi-region access point or an

+// Object Lambda access point names no bucket in its ARN and can front

+// any bucket's objects: its principal is listed as possibly reaching,

+// marked so, never dropped.

// What it cannot read is an error, never a shorter list: a document it

-// cannot decode; a statement or an element that is not the identity

-// policy grammar it knows; the same principal, policy or inline policy

-// name returned twice; an attached managed policy the listing did not

-// return; a listing truncated without a marker, or that does not end.

+// cannot decode; a JSON member given twice anywhere in a document; a

+// statement or an element that is not the grammar it knows, in an

+// identity policy or a trust policy, Deny statements included; the same

+// principal, policy or inline policy name returned twice; a managed

+// policy whose own default-version id and the version marked default

+// disagree; an attached managed policy the listing did not return; a

+// listing truncated without a marker, or that does not end.

// What it does not read at all: the root user; a bucket's own policy, ACL

-// and access points; grants made through other services' resources

-// (access points, Object Lambda, Batch Operations and replication jobs);

-// anything in another account.

+// and its access points' policies; what Batch Operations and replication

+// jobs may do under their roles; anything in another account.

// The platform's own. A deployment may name principals that are its own,

// by exact ARN; the report sets them apart. That is a label for the

@@ -281,15 +288,23 @@ func (p *PrincipalReport) listAccount(ctx context.Context) (*accountPolicies, er

}

defaults := 0

var current *string

+ var currentID string

for _, v := range m.PolicyVersionList {

if v.IsDefaultVersion {

defaults++

current = v.Document

+ currentID = aws.ToString(v.VersionId)

}

if defaults != 1 || current == nil {

return nil, fmt.Errorf("objstore: managed policy %s has %d default versions with a document in the listing; want one", arn, defaults)

}

+ // The policy's own record of its default version must name

+ // the version marked default: two parts of one answer that

+ // disagree are not an answer.

+ if want := aws.ToString(m.DefaultVersionId); want == "" || currentID == "" || want != currentID {

+ return nil, fmt.Errorf("objstore: managed policy %s says its default version is %q and the listing marks %q", arn, want, currentID)

+ }

doc, err := decodePolicy(aws.ToString(current))

if err != nil {

return nil, fmt.Errorf("objstore: managed policy %s: %w", arn, err)

@@ -318,19 +333,23 @@ func (a *accountPolicies) reaching(bucket string, platform map[string]bool) (*Pr

partition = parts[1]

}

var out []string

- for name, doc := range pr.inline {

- if doc.reaches(partition, bucket) {

- out = append(out, "inline:"+name)

+ note := func(kind reachKind, what string) {

+ switch kind {

+ case reachDirect:

+ out = append(out, what)

+ case reachPossible:

+ out = append(out, "possibly, through an access point or Object Lambda: "+what)

}

+ for name, doc := range pr.inline {

+ note(doc.reaches(partition, bucket), "inline:"+name)

+ }

for _, arn := range pr.attached {

doc, ok := a.managed[arn]

if !ok {

return nil, fmt.Errorf("objstore: %s has managed policy %s attached, and the listing did not return it", pr.arn, arn)

}

- if doc.reaches(partition, bucket) {

- out = append(out, "managed:"+arn)

- }

+ note(doc.reaches(partition, bucket), "managed:"+arn)

}

return out, nil

}

@@ -428,6 +447,58 @@ func stringOrList(raw json.RawMessage) ([]string, error) {

return out, nil

}

+// noRepeatedMembers refuses a JSON text in which any object, at any

+// depth, gives one member name twice: the standard decoder keeps the last,

+// and the first may be the one that reaches.

+func noRepeatedMembers(text string) error {

+ dec := json.NewDecoder(strings.NewReader(text))

+ type frame struct {

+ object bool

+ seen map[string]bool

+ expectKey bool

+ }

+ var stack []*frame

+ for {

+ tok, err := dec.Token()

+ if err == io.EOF {

+ return nil

+ }

+ if err != nil {

+ return err

+ }

+ if d, isDelim := tok.(json.Delim); isDelim && (d == '}' || d == ']') {

+ stack = stack[:len(stack)-1]

+ if len(stack) > 0 && stack[len(stack)-1].object {

+ stack[len(stack)-1].expectKey = true

+ }

+ continue

+ }

+ if len(stack) > 0 {

+ top := stack[len(stack)-1]

+ if top.object && top.expectKey {

+ name, _ := tok.(string)

+ if top.seen[name] {

+ return fmt.Errorf("the member %q is given twice", name)

+ }

+ top.seen[name] = true

+ top.expectKey = false

+ continue

+ }

+ // A value: a scalar, or the start of an object or a list.

+ if d, isDelim := tok.(json.Delim); isDelim {

+ if len(stack) > 0 && stack[len(stack)-1].object {

+ stack[len(stack)-1].expectKey = true

+ }

+ stack = append(stack, &frame{object: d == '{', seen: map[string]bool{}, expectKey: d == '{'})

+ continue

+ }

+ if len(stack) > 0 && stack[len(stack)-1].object {

+ stack[len(stack)-1].expectKey = true

+ }

// policyText turns a policy document as IAM returns it (percent-encoded

// JSON, RFC 3986: a plus sign is a plus sign; or plain JSON) into its

// top-level elements, and checks them against the ones a policy has.

@@ -443,6 +514,9 @@ func policyText(raw string) (map[string]json.RawMessage, error) {

}

text = decoded

}

+ if err := noRepeatedMembers(text); err != nil {

+ return nil, fmt.Errorf("policy document: %w", err)

+ }

var doc map[string]json.RawMessage

if err := json.Unmarshal([]byte(text), &doc); err != nil || doc == nil {

return nil, errors.New("policy document is not a JSON object")

@@ -460,9 +534,43 @@ func policyText(raw string) (map[string]json.RawMessage, error) {

return nil, fmt.Errorf("policy document has the version %s", string(v))

}

+ if v, ok := doc["Id"]; ok {

+ var id string

+ if err := json.Unmarshal(v, &id); err != nil {

+ return nil, fmt.Errorf("policy document has the id %s", string(v))

+ }

return doc, nil

}

+// statementShape checks the elements of one statement, of an identity

+// policy or a trust policy, that are not evaluated: their shapes must be

+// the grammar's even though their meaning is left alone. A statement is

+// read in full whatever its effect.

+func statementShape(i int, st map[string]json.RawMessage, allowed map[string]bool) error {

+ for key, raw := range st {

+ if !allowed[key] {

+ if key == "Principal" || key == "NotPrincipal" {

+ return fmt.Errorf("statement %d has %s, which an identity policy does not", i, key)

+ }

+ return fmt.Errorf("statement %d has the element %q", i, key)

+ }

+ switch key {

+ case "Sid":

+ var sid string

+ if err := json.Unmarshal(raw, &sid); err != nil {

+ return fmt.Errorf("statement %d has the sid %s", i, string(raw))

+ }

+ case "Condition":

+ var cond map[string]map[string]json.RawMessage

+ if err := json.Unmarshal(raw, &cond); err != nil || cond == nil {

+ return fmt.Errorf("statement %d has a condition that is not a map of operators", i)

+ }

+ return nil

// policyStatements gives a document's statements, each as its elements:

// one statement or a list of at least one, never null.

func policyStatements(doc map[string]json.RawMessage) ([]map[string]json.RawMessage, error) {

@@ -510,15 +618,10 @@ func decodePolicy(raw string) (*policyDoc, error) {

return nil, err

}

out := &policyDoc{}

+ identity := map[string]bool{"Sid": true, "Effect": true, "Action": true, "NotAction": true, "Resource": true, "NotResource": true, "Condition": true}

for i, st := range list {

- for key := range st {

- switch key {

- case "Sid", "Effect", "Action", "NotAction", "Resource", "NotResource", "Condition":

- case "Principal", "NotPrincipal":

- return nil, fmt.Errorf("statement %d has %s, which an identity policy does not", i, key)

- default:

- return nil, fmt.Errorf("statement %d has the element %q", i, key)

- }

+ if err := statementShape(i, st, identity); err != nil {

+ return nil, err

}

allow, err := statementEffect(i, st)

if err != nil {

@@ -570,46 +673,90 @@ func trustPrincipals(raw string) ([]string, error) {

return nil, err

}

seen := map[string]bool{}

+ trust := map[string]bool{"Sid": true, "Effect": true, "Principal": true, "NotPrincipal": true, "Action": true, "NotAction": true, "Condition": true}

for i, st := range list {

+ // Every statement is read in full, a Deny included: its shape is

+ // the grammar's or the document is not read.

+ if err := statementShape(i, st, trust); err != nil {

+ return nil, err

+ }

allow, err := statementEffect(i, st)

if err != nil {

return nil, err

}

- if !allow {

- continue

+ _, hasAction := st["Action"]

+ _, hasNotAction := st["NotAction"]

+ if hasAction == hasNotAction {

+ return nil, fmt.Errorf("statement %d does not have exactly one of Action and NotAction", i)

}

- if _, not := st["NotPrincipal"]; not {

- seen["*"] = true

- continue

+ for _, key := range []string{"Action", "NotAction"} {

+ if raw, ok := st[key]; ok {

+ if _, err := stringOrList(raw); err != nil {

+ return nil, fmt.Errorf("statement %d %s: %w", i, key, err)

+ }

}

- rawPrincipal, ok := st["Principal"]

- if !ok {

- return nil, fmt.Errorf("statement %d names no principal", i)

+ rawPrincipal, hasPrincipal := st["Principal"]

+ rawNot, hasNot := st["NotPrincipal"]

+ if hasPrincipal == hasNot {

+ return nil, fmt.Errorf("statement %d does not have exactly one of Principal and NotPrincipal", i)

+ }

+ // The principal element's shape, whichever it is: "*", or a map

+ // of the kinds a trust policy names, each a string or a list of

+ // strings, none empty.

+ principals := map[string]bool{}

+ readPrincipal := func(raw json.RawMessage) error {

+ var star string

+ if json.Unmarshal(raw, &star) == nil {

+ if star != "*" {

+ return fmt.Errorf("statement %d has the principal %q", i, star)

+ }

+ principals["*"] = true

+ return nil

+ }

+ var kinds map[string]json.RawMessage

+ if err := json.Unmarshal(raw, &kinds); err != nil || len(kinds) == 0 {

+ return fmt.Errorf("statement %d has a principal that is neither \"*\" nor a map of kinds", i)

+ }

+ for kind, rawValues := range kinds {

+ switch kind {

+ case "AWS", "Service", "Federated", "CanonicalUser":

+ default:

+ return fmt.Errorf("statement %d has the principal kind %q", i, kind)

+ }

+ values, err := stringOrList(rawValues)

+ if err != nil {

+ return fmt.Errorf("statement %d principal %s: %w", i, kind, err)

+ }

+ for _, v := range values {

+ if v == "*" {

+ principals["*"] = true

+ } else {

+ principals[kind+":"+v] = true

+ }

+ return nil

}

- var star string

- if json.Unmarshal(rawPrincipal, &star) == nil {

- if star != "*" {

- return nil, fmt.Errorf("statement %d has the principal %q", i, star)

+ if hasPrincipal {

+ if err := readPrincipal(rawPrincipal); err != nil {

+ return nil, err

+ }

+ } else {

+ if err := readPrincipal(rawNot); err != nil {

+ return nil, err

}

+ }

+ if !allow {

+ continue

+ }

+ if hasNot {

+ // Everyone but the named: anyone, as far as this report says.

seen["*"] = true

continue

}

- var kinds map[string]json.RawMessage

- if err := json.Unmarshal(rawPrincipal, &kinds); err != nil || len(kinds) == 0 {

- return nil, fmt.Errorf("statement %d has a principal that is not a map of kinds", i)

- }

- for kind, rawValues := range kinds {

- values, err := stringOrList(rawValues)

- if err != nil {

- return nil, fmt.Errorf("statement %d principal %s: %w", i, kind, err)

- }

- for _, v := range values {

- if v == "*" {

- seen["*"] = true

- } else {

- seen[kind+":"+v] = true

- }

+ for v := range principals {

+ seen[v] = true

}

out := make([]string, 0, len(seen))

@@ -620,33 +767,93 @@ func trustPrincipals(raw string) ([]string, error) {

return out, nil

}

+// reachKind is how a document reaches a bucket: directly (the bucket's

+// ARN or an object under it), or possibly, through a resource that can

+// front a bucket's objects and whose relation to this bucket the report

+// cannot see (an access point, a multi-region access point, an Object

+// Lambda access point). A possible reach is reported, never dropped.

+type reachKind int

+const (

+ reachNone reachKind = iota

+ reachPossible

+ reachDirect

// reaches reports whether any Allow statement of the document can apply

-// an S3 action to the bucket or to an object under it.

-func (d *policyDoc) reaches(partition, bucket string) bool {

+// an S3 action to the bucket or to an object under it, directly or

+// possibly.

+func (d *policyDoc) reaches(partition, bucket string) reachKind {

arn := "arn:" + partition + ":s3:::" + bucket

+ kind := reachNone

for _, s := range d.statements {

if !s.allow {

continue

}

- action := s.hasNotAction

+ action, lambda := s.hasNotAction, s.hasNotAction

for _, a := range s.actions {

if actionCanBeS3(a) {

action = true

}

+ if actionCanBeObjectLambda(a) {

+ lambda = true

+ }

}

- if !action {

+ if !action && !lambda {

continue

}

if s.hasNotResource {

- return true

+ if action {

+ return reachDirect

+ }

+ kind = max(kind, reachPossible)

+ continue

}

for _, r := range s.resources {

- if resourceReaches(r, arn) {

- return true

+ if action && resourceReaches(r, arn) {

+ return reachDirect

+ }

+ if resourceIsAnAccessPath(r, partition, action, lambda) {

+ kind = max(kind, reachPossible)

}

- return false

+ return kind

+// resourceIsAnAccessPath reports whether a resource pattern can name an

+// access point (arn:<p>:s3:<region>:<account>:accesspoint/...), a

+// multi-region access point (arn:<p>:s3::<account>:accesspoint/...) or an

+// Object Lambda access point (arn:<p>:s3-object-lambda:...:accesspoint/...),

+// through which a bucket's objects can be reached. Which bucket is not in

+// the ARN, so any of them is a possible reach of every bucket. The ARN is

+// read field by field; a pattern with too few fields (a star standing for

+// several) is taken to be able to name one.

+func resourceIsAnAccessPath(pattern, partition string, s3, lambda bool) bool {

+ p := conservativePattern(pattern)

+ parts := strings.SplitN(p, ":", 6)

+ if len(parts) < 6 {

+ return strings.Contains(p, "*")

+ }

+ if !wildcardMatch(parts[0], "arn") || !wildcardMatch(parts[1], partition) {

+ return false

+ }

+ accessPoint := wildcardLive(parts[5], "accesspoint/") != nil

+ if !accessPoint {

+ return false

+ }

+ return (s3 && wildcardMatch(parts[2], "s3")) || (lambda && wildcardMatch(parts[2], "s3-object-lambda"))

+// actionCanBeObjectLambda reports whether an action pattern can name an

+// action of the Object Lambda service.

+func actionCanBeObjectLambda(pattern string) bool {

+ p := strings.ToLower(conservativePattern(pattern))

+ service, _, ok := strings.Cut(p, ":")

+ if !ok {

+ return wildcardLive(p, "s3-object-lambda:") != nil

+ }

+ return wildcardMatch(service, "s3-object-lambda")

}

// actionCanBeS3 reports whether an action pattern can name an action of

diff --git a/internal/objstore/principals_test.go b/internal/objstore/principals_test.go

index c83194d33..34c3cdf62 100644

--- a/internal/objstore/principals_test.go

+++ b/internal/objstore/principals_test.go

@@ -81,7 +81,7 @@ func invGroup(name string, inline map[string]string, attached ...string) iamtype

}

func invManaged(arn, doc string) iamtypes.ManagedPolicyDetail {

- return iamtypes.ManagedPolicyDetail{Arn: aws.String(arn), PolicyVersionList: []iamtypes.PolicyVersion{

+ return iamtypes.ManagedPolicyDetail{Arn: aws.String(arn), DefaultVersionId: aws.String("v2"), PolicyVersionList: []iamtypes.PolicyVersion{

{VersionId: aws.String("v1"), Document: encoded(allow(`"s3:*"`, `"*"`))}, // an old version that reaches everything: not the default, not read

{VersionId: aws.String("v2"), IsDefaultVersion: true, Document: encoded(doc)},

}}

@@ -147,7 +147,7 @@ func TestPolicyPatternsThatReachABucket(t *testing.T) {

t.Errorf("%s: %v", tc.name, err)

continue

}

- if got := doc.reaches("aws", bucket); got != tc.want {

+ if got := doc.reaches("aws", bucket) == reachDirect; got != tc.want {

t.Errorf("%s: reaches %v, want %v", tc.name, got, tc.want)

}

@@ -172,40 +172,48 @@ func TestPolicyPatternsThatReachABucket(t *testing.T) {

t.Errorf("%s: %v", name, err)

continue

}

- if got := doc.reaches("aws", bucket); got != tc.want {

+ if got := doc.reaches("aws", bucket) == reachDirect; got != tc.want {

t.Errorf("%s: reaches %v, want %v", name, got, tc.want)

}

// What it cannot read is an error, never "does not reach".

for name, doc := range map[string]string{

- "an unknown effect": `{"Statement":[{"Effect":"Permit","Action":"s3:*","Resource":"*"}]}`,

- "an effect in the wrong case": `{"Statement":[{"Effect":"allow","Action":"s3:*","Resource":"*"}]}`,

- "an Allow with no resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*"}]}`,

- "an Allow with no action": `{"Statement":[{"Effect":"Allow","Resource":"*"}]}`,

- "both Resource and NotResource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","NotResource":"*"}]}`,

- "a resource that is a number": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":7}]}`,

- "a statement that is null": `{"Statement":null}`,

- "no statements": `{"Statement":[]}`,

- "a null among the statements": `{"Statement":[null]}`,

- "no actions": `{"Statement":[{"Effect":"Allow","Action":[],"Resource":"*"}]}`,

- "no resources": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":[]}]}`,

- "an action that is null": `{"Statement":[{"Effect":"Allow","Action":null,"Resource":"*"}]}`,

- "a null among the resources": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":["arn:aws:s3:::x",null]}]}`,

- "an empty resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":""}]}`,

- "a principal, as a bucket policy has": `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:*","Resource":"*"}]}`,

- "a NotPrincipal": `{"Statement":[{"Effect":"Deny","NotPrincipal":{"AWS":"*"},"Action":"s3:*","Resource":"*"}]}`,

- "an element it does not know": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","Scope":"all"}]}`,

- "a version it does not know": `{"Version":"2030-01-01","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`,

- "a version that is a number": `{"Version":2012,"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`,

- "a top-level element it does not know": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}],"Extra":1}`,

- "a Deny with no resource": `{"Statement":[{"Effect":"Deny","Action":"s3:*"}]}`,

- "a document that is a list": `[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]`,

- "null": `null`,

- "no statement": `{"Version":"2012-10-17"}`,

- "not JSON": `Statement: none`,

- "an empty document": ``,

- "a broken percent-encoding": `%7B%2`,

- "a statement that is a string": `{"Statement":"Allow"}`,

+ "an unknown effect": `{"Statement":[{"Effect":"Permit","Action":"s3:*","Resource":"*"}]}`,

+ "an effect in the wrong case": `{"Statement":[{"Effect":"allow","Action":"s3:*","Resource":"*"}]}`,

+ "an Allow with no resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*"}]}`,

+ "an Allow with no action": `{"Statement":[{"Effect":"Allow","Resource":"*"}]}`,

+ "both Resource and NotResource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","NotResource":"*"}]}`,

+ "a resource that is a number": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":7}]}`,

+ "a statement that is null": `{"Statement":null}`,

+ "no statements": `{"Statement":[]}`,

+ "a null among the statements": `{"Statement":[null]}`,

+ "no actions": `{"Statement":[{"Effect":"Allow","Action":[],"Resource":"*"}]}`,

+ "no resources": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":[]}]}`,

+ "an action that is null": `{"Statement":[{"Effect":"Allow","Action":null,"Resource":"*"}]}`,

+ "a null among the resources": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":["arn:aws:s3:::x",null]}]}`,

+ "an empty resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":""}]}`,

+ "a principal, as a bucket policy has": `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:*","Resource":"*"}]}`,

+ "a NotPrincipal": `{"Statement":[{"Effect":"Deny","NotPrincipal":{"AWS":"*"},"Action":"s3:*","Resource":"*"}]}`,

+ "an element it does not know": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","Scope":"all"}]}`,

+ "a version it does not know": `{"Version":"2030-01-01","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`,

+ "a version that is a number": `{"Version":2012,"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`,

+ "a top-level element it does not know": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}],"Extra":1}`,

+ "a Deny with no resource": `{"Statement":[{"Effect":"Deny","Action":"s3:*"}]}`,

+ "a document that is a list": `[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]`,

+ "a member given twice, the second harmless": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}],"Statement":[{"Effect":"Allow","Action":"ec2:*","Resource":"*"}]}`,

+ "a member given twice inside a statement": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Action":"ec2:*","Resource":"*"}]}`,

+ "a condition that is a number": `{"Statement":[{"Effect":"Allow","Action":"ec2:*","Resource":"*","Condition":7}]}`,

+ "a condition that is a list": `{"Statement":[{"Effect":"Allow","Action":"ec2:*","Resource":"*","Condition":["x"]}]}`,

+ "a sid that is a number": `{"Statement":[{"Sid":1,"Effect":"Allow","Action":"ec2:*","Resource":"*"}]}`,

+ "an id that is a number": `{"Id":1,"Statement":[{"Effect":"Allow","Action":"ec2:*","Resource":"*"}]}`,

+ "a Deny with an unknown element": `{"Statement":[{"Effect":"Deny","Action":"s3:*","Resource":"*","Unexpected":true}]}`,

+ "a Deny with a null action": `{"Statement":[{"Effect":"Deny","Action":null,"Resource":"*"}]}`,

+ "null": `null`,

+ "no statement": `{"Version":"2012-10-17"}`,

+ "not JSON": `Statement: none`,

+ "an empty document": ``,

+ "a broken percent-encoding": `%7B%2`,

+ "a statement that is a string": `{"Statement":"Allow"}`,

} {

if _, err := decodePolicy(doc); err == nil {

t.Errorf("%s: decoded; want an error", name)

@@ -213,7 +221,7 @@ func TestPolicyPatternsThatReachABucket(t *testing.T) {

}

// A plus sign in an encoded document is a plus sign, not a space.

doc, err := decodePolicy(`%7B%22Statement%22%3A%5B%7B%22Effect%22%3A%22Allow%22%2C%22Action%22%3A%22s3%3A%2A%22%2C%22Resource%22%3A%22arn%3Aaws%3As3%3A%3A%3Aa+b%22%7D%5D%7D`)

- if err != nil || !doc.reaches("aws", "a+b") || doc.reaches("aws", "a b") {

+ if err != nil || doc.reaches("aws", "a+b") != reachDirect || doc.reaches("aws", "a b") != reachNone {

t.Errorf("an encoded plus sign: %v", err)

}

@@ -361,12 +369,20 @@ func TestInventoryReportsWhoARoleTrusts(t *testing.T) {

}

// A trust policy it cannot read is an error, not a role with nobody.

for name, doc := range map[string]string{

- "no principal": `{"Statement":[{"Effect":"Allow","Action":"sts:AssumeRole"}]}`,

- "a principal that is a word": `{"Statement":[{"Effect":"Allow","Principal":"everyone","Action":"sts:AssumeRole"}]}`,

- "an empty principal map": `{"Statement":[{"Effect":"Allow","Principal":{},"Action":"sts:AssumeRole"}]}`,

- "a null principal value": `{"Statement":[{"Effect":"Allow","Principal":{"AWS":null},"Action":"sts:AssumeRole"}]}`,

- "no trust policy": ``,

- "an unknown effect": `{"Statement":[{"Effect":"Maybe","Principal":"*"}]}`,

+ "no principal": `{"Statement":[{"Effect":"Allow","Action":"sts:AssumeRole"}]}`,

+ "a principal that is a word": `{"Statement":[{"Effect":"Allow","Principal":"everyone","Action":"sts:AssumeRole"}]}`,

+ "an empty principal map": `{"Statement":[{"Effect":"Allow","Principal":{},"Action":"sts:AssumeRole"}]}`,

+ "a null principal value": `{"Statement":[{"Effect":"Allow","Principal":{"AWS":null},"Action":"sts:AssumeRole"}]}`,

+ "no trust policy": ``,

+ "an unknown effect": `{"Statement":[{"Effect":"Maybe","Principal":"*"}]}`,

+ "a Deny with an unknown element": `{"Statement":{"Effect":"Deny","Principal":"*","Action":"sts:AssumeRole","Unexpected":true}}`,

+ "a Deny with a null principal": `{"Statement":{"Effect":"Deny","Principal":null,"Action":"sts:AssumeRole"}}`,

+ "an Allow with a null action": `{"Statement":[{"Effect":"Allow","Principal":"*","Action":null}]}`,

+ "an Allow with no action": `{"Statement":[{"Effect":"Allow","Principal":"*"}]}`,

+ "an unknown principal kind": `{"Statement":[{"Effect":"Allow","Principal":{"Anyone":"*"},"Action":"sts:AssumeRole"}]}`,

+ "a NotPrincipal that is null": `{"Statement":[{"Effect":"Allow","NotPrincipal":null,"Action":"sts:AssumeRole"}]}`,

+ "both Principal and NotPrincipal": `{"Statement":[{"Effect":"Allow","Principal":"*","NotPrincipal":{"AWS":"x"},"Action":"sts:AssumeRole"}]}`,

+ "a member given twice": `{"Statement":[{"Effect":"Allow","Principal":{"AWS":"arn:aws:iam::999999999999:root"},"Principal":{"Service":"ec2.amazonaws.com"},"Action":"sts:AssumeRole"}]}`,

} {

p, _ := inventoryFor(t, nil, &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{role("r", doc)}})

if inv, err := p.BucketPrincipals(context.Background(), "carlos-store-bac-blog"); err == nil {

@@ -375,6 +391,50 @@ func TestInventoryReportsWhoARoleTrusts(t *testing.T) {

}

+// A grant on an access point, a multi-region access point or an Object

+// Lambda access point names no bucket and can front any bucket's objects:

+// its principal is reported as possibly reaching, marked so. A grant on

+// another service's resource is not.

+func TestAnAccessPointGrantIsAPossibleReach(t *testing.T) {

+ const bucket = "carlos-store-bac-blog"

+ for name, tc := range map[string]struct {

+ doc string

+ want reachKind

+ }{

+ "an access point's objects": {allow(`"s3:GetObject"`, `"arn:aws:s3:eu-west-1:123456789012:accesspoint/readers/object/*"`), reachPossible},

+ "an access point itself": {allow(`"s3:*"`, `"arn:aws:s3:eu-west-1:123456789012:accesspoint/readers"`), reachPossible},

+ // (A star in the account field can, read as text, run into the

+ // resource, so this pattern also reaches an object whose key holds

+ // ":accesspoint/": a direct reach, which is the stronger answer.)

+ "any access point": {allow(`"s3:GetObject"`, `"arn:aws:s3:*:*:accesspoint/*"`), reachDirect},

+ "a multi-region access point": {allow(`"s3:GetObject"`, `"arn:aws:s3::123456789012:accesspoint/mfzwi23gnjvgw.mrap/object/*"`), reachPossible},

+ "an Object Lambda access point": {allow(`"s3-object-lambda:GetObject"`, `"arn:aws:s3-object-lambda:eu-west-1:123456789012:accesspoint/thumbs"`), reachPossible},

+ "Object Lambda on everything": {allow(`"s3-object-lambda:*"`, `"*"`), reachPossible},

+ "the bucket itself beside an access point": {allow(`"s3:GetObject"`, `["arn:aws:s3:eu-west-1:123456789012:accesspoint/readers/object/*","arn:aws:s3:::carlos-store-bac-blog/*"]`), reachDirect},

+ "another service's resource": {allow(`"s3:GetObject"`, `"arn:aws:lambda:eu-west-1:123456789012:function:thumbs"`), reachNone},

+ "an S3 action on an access point in another partition": {allow(`"s3:GetObject"`, `"arn:aws-cn:s3:cn-north-1:123456789012:accesspoint/readers/object/*"`), reachNone},

+ "an EC2 action on an access point ARN": {allow(`"ec2:*"`, `"arn:aws:s3:eu-west-1:123456789012:accesspoint/readers"`), reachNone},

+ "S3 on Outposts": {allow(`"s3-outposts:GetObject"`, `"arn:aws:s3-outposts:eu-west-1:123456789012:outpost/op-1/accesspoint/ap/object/*"`), reachNone},

+ } {

+ doc, err := decodePolicy(tc.doc)

+ if err != nil {

+ t.Errorf("%s: %v", name, err)

+ continue

+ }

+ if got := doc.reaches("aws", bucket); got != tc.want {

+ t.Errorf("%s: reaches %v, want %v", name, got, tc.want)

+ }

+ // In the report: set apart in the Via, never dropped.

+ p, _ := inventoryFor(t, nil, &iam.GetAccountAuthorizationDetailsOutput{UserDetailList: []iamtypes.UserDetail{

+ invUser("through-an-access-point", nil, map[string]string{"ap": allow(`"s3:GetObject"`, `"arn:aws:s3:eu-west-1:123456789012:accesspoint/readers/object/*"`)}),

+ }})

+ inv, err := p.BucketPrincipals(context.Background(), bucket)

+ if err != nil || len(inv.Reaching) != 1 || len(inv.Reaching[0].Via) != 1 || !strings.HasPrefix(inv.Reaching[0].Via[0], "possibly, through an access point or Object Lambda: inline:ap") {

+ t.Fatalf("the report: %+v %v", inv, err)

+ }

// A principal's policies are matched in the principal's own partition: a

// bucket ARN is spelled with it.

func TestInventoryMatchesInThePrincipalsPartition(t *testing.T) {

@@ -462,7 +522,11 @@ func TestInventoryClaimsNothingItCannotRead(t *testing.T) {

}

for name, pages := range map[string][]*iam.GetAccountAuthorizationDetailsOutput{

"a managed policy attached and not listed": {{RoleDetailList: []iamtypes.RoleDetail{invRole("r", nil, "arn:aws:iam::123456789012:policy/missing")}}},

- "a managed policy with no default version": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{{VersionId: aws.String("v1"), Document: encoded(harmless)}}}}}},

+ "a managed policy with no default version": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), DefaultVersionId: aws.String("v1"), PolicyVersionList: []iamtypes.PolicyVersion{{VersionId: aws.String("v1"), Document: encoded(harmless)}}}}}},

+ "a managed policy whose default id names another version": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), DefaultVersionId: aws.String("v2"), PolicyVersionList: []iamtypes.PolicyVersion{

+ {VersionId: aws.String("v1"), IsDefaultVersion: true, Document: encoded(harmless)}, {VersionId: aws.String("v2"), Document: encoded(reaches)}}}}}},

+ "a managed policy with no default id": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{{VersionId: aws.String("v1"), IsDefaultVersion: true, Document: encoded(harmless)}}}}}},

+ "a managed policy whose default version has no id": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), DefaultVersionId: aws.String("v1"), PolicyVersionList: []iamtypes.PolicyVersion{{IsDefaultVersion: true, Document: encoded(harmless)}}}}}},

"a managed policy with two default versions": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{

{VersionId: aws.String("v1"), IsDefaultVersion: true, Document: encoded(harmless)}, {VersionId: aws.String("v2"), IsDefaultVersion: true, Document: encoded(harmless)}}}}}},

"two default versions, the first with no document": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{