mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-11-02 09:13:33 +00:00
fix: refuse ECR policy texts that are omitted or outside the model's length before parsing them (#1402)
This commit is contained in:
@@ -52,7 +52,11 @@ func EvaluateLifecyclePreview(ctx context.Context, policies LifecyclePolicyStore
|
||||
}
|
||||
|
||||
policyText := req.LifecyclePolicyText
|
||||
if policyText == "" {
|
||||
if policyText != "" {
|
||||
if err := ValidateTextLength("lifecyclePolicyText", policyText, minLifecyclePolicyTextLength, maxLifecyclePolicyTextLength); err != nil {
|
||||
return LifecyclePreview{}, err
|
||||
}
|
||||
} else {
|
||||
stored, err := policies.GetLifecyclePolicy(ctx, accountID, req.RepositoryName)
|
||||
if err != nil {
|
||||
if errors.Is(err, ecrdomain.ErrNotFound) {
|
||||
|
||||
@@ -3,6 +3,7 @@ package awsapi
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -69,8 +70,8 @@ func TestEvaluateLifecyclePreview_MapsPolicyAndRepositoryFailures(t *testing.T)
|
||||
code string
|
||||
}{
|
||||
{"missing policy", fakeLifecyclePolicyStore{err: ecrdomain.ErrNotFound}, fakeImageCatalog{}, `{"repositoryName":"team/app"}`, awserrors.ErrorLifecyclePolicyNotFound},
|
||||
{"missing repository", fakeLifecyclePolicyStore{}, fakeImageCatalog{err: ecrdomain.ErrNotFound}, `{"repositoryName":"team/app","lifecyclePolicyText":"{}"}`, awserrors.ErrorRepositoryNotFound},
|
||||
{"invalid policy", fakeLifecyclePolicyStore{}, fakeImageCatalog{}, `{"repositoryName":"team/app","lifecyclePolicyText":"not-json"}`, awserrors.ErrorECRInvalidParameter},
|
||||
{"missing repository", fakeLifecyclePolicyStore{}, fakeImageCatalog{err: ecrdomain.ErrNotFound}, `{"repositoryName":"team/app","lifecyclePolicyText":` + strconvQuote(lifecyclePreviewPolicy) + `}`, awserrors.ErrorRepositoryNotFound},
|
||||
{"invalid policy", fakeLifecyclePolicyStore{}, fakeImageCatalog{}, `{"repositoryName":"team/app","lifecyclePolicyText":` + strconvQuote("not-json"+strings.Repeat(" ", 100)) + `}`, awserrors.ErrorECRInvalidParameter},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
@@ -80,3 +81,26 @@ func TestEvaluateLifecyclePreview_MapsPolicyAndRepositoryFailures(t *testing.T)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A parseable policy outside the model's 100-30720 characters is refused on
|
||||
// length, and an omitted text still falls back to the stored policy.
|
||||
func TestEvaluateLifecyclePreview_PolicyTextLength(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
text string
|
||||
message string
|
||||
}{
|
||||
{"too short", `{"rules":[]}`, "Invalid parameter at 'lifecyclePolicyText' failed to satisfy constraint: 'Member must have length greater than or equal to 100'"},
|
||||
{"too long", lifecyclePreviewPolicy + strings.Repeat(" ", 30720), "Invalid parameter at 'lifecyclePolicyText' failed to satisfy constraint: 'Member must have length less than or equal to 30720'"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
body := []byte(`{"repositoryName":"team/app","lifecyclePolicyText":` + strconvQuote(tc.text) + `}`)
|
||||
_, err := EvaluateLifecyclePreview(context.Background(), fakeLifecyclePolicyStore{}, fakeImageCatalog{}, "123456789012", body)
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorECRInvalidParameter, code)
|
||||
assert.Equal(t, tc.message, message)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,6 +71,12 @@ func PutLifecyclePolicy(ctx context.Context, nc *nats.Conn, accountID string, bo
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if req.LifecyclePolicyText == "" {
|
||||
return nil, RequiredParameterError("lifecyclePolicyText")
|
||||
}
|
||||
if err := ValidateTextLength("lifecyclePolicyText", req.LifecyclePolicyText, minLifecyclePolicyTextLength, maxLifecyclePolicyTextLength); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if _, err := ecrdomain.ParseLifecyclePolicy([]byte(req.LifecyclePolicyText)); err != nil {
|
||||
return nil, InvalidLifecyclePolicyError()
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package awsapi
|
||||
import (
|
||||
"context"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -73,7 +74,7 @@ func TestLifecyclePolicy_Errors(t *testing.T) {
|
||||
expect string
|
||||
}{
|
||||
{"put missing repo", PutLifecyclePolicy, `{"repositoryName":"team/ghost","lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorRepositoryNotFound},
|
||||
{"put invalid json", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":"not-json"}`, awserrors.ErrorECRInvalidParameter},
|
||||
{"put invalid json", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":` + strconvQuote("not-json"+strings.Repeat(" ", 100)) + `}`, awserrors.ErrorECRInvalidParameter},
|
||||
{"put bad rule", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":"{\"rules\":[{\"rulePriority\":1,\"selection\":{\"tagStatus\":\"tagged\",\"countType\":\"imageCountMoreThan\",\"countNumber\":1},\"action\":{\"type\":\"expire\"}}]}"}`, awserrors.ErrorECRInvalidParameter},
|
||||
{"put empty name", PutLifecyclePolicy, `{"lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorECRInvalidParameter},
|
||||
{"put cross-account", PutLifecyclePolicy, `{"repositoryName":"team/app","registryId":"999999999999","lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorAccessDenied},
|
||||
@@ -144,3 +145,29 @@ func TestLifecyclePolicy_NotFoundMessagesNameRepository(t *testing.T) {
|
||||
assert.Equal(t, awserrors.ErrorRepositoryNotFound, code)
|
||||
assert.Equal(t, "The repository with name 'team/ghost' does not exist in the registry with id '"+policyTestAccount+"'", message)
|
||||
}
|
||||
|
||||
// The text is checked for presence and the model's 100-30720 characters before
|
||||
// it is parsed, so a parseable policy outside those bounds is still refused.
|
||||
func TestPutLifecyclePolicy_PolicyTextRequiredAndLength(t *testing.T) {
|
||||
nc := newLifecycleTestConn(t)
|
||||
seedRepo(t, nc, "team/app")
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
body string
|
||||
message string
|
||||
}{
|
||||
{"omitted", `{"repositoryName":"team/app"}`, "Invalid parameter at 'lifecyclePolicyText' failed to satisfy constraint: 'Member must not be null'"},
|
||||
{"too short", `{"repositoryName":"team/app","lifecyclePolicyText":"{\"rules\":[]}"}`, "Invalid parameter at 'lifecyclePolicyText' failed to satisfy constraint: 'Member must have length greater than or equal to 100'"},
|
||||
{"too long", `{"repositoryName":"team/app","lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy+strings.Repeat(" ", 30720)) + `}`, "Invalid parameter at 'lifecyclePolicyText' failed to satisfy constraint: 'Member must have length less than or equal to 30720'"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
_, err := PutLifecyclePolicy(context.Background(), nc, policyTestAccount, []byte(tc.body))
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorECRInvalidParameter, code)
|
||||
assert.Equal(t, tc.message, message)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -61,6 +61,9 @@ func SetRepositoryPolicy(ctx context.Context, nc *nats.Conn, accountID string, b
|
||||
if req.PolicyText == "" {
|
||||
return nil, ConstraintError("PolicyText", "Cannot be null")
|
||||
}
|
||||
if err := ValidateTextLength("policyText", req.PolicyText, 0, maxRepositoryPolicyTextLength); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if !json.Valid([]byte(req.PolicyText)) {
|
||||
return nil, ConstraintError("policyText", "Invalid repository policy provided")
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"github.com/mulgadc/spinifex/spinifex/foundation/messaging/nats"
|
||||
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -136,6 +137,18 @@ func TestSetRepositoryPolicy_MissingPolicyTextMessage(t *testing.T) {
|
||||
assert.Equal(t, "Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Cannot be null'", message)
|
||||
}
|
||||
|
||||
// A valid policy longer than the model's 10240 characters is refused on length.
|
||||
func TestSetRepositoryPolicy_PolicyTextTooLong(t *testing.T) {
|
||||
nc := newPolicyTestConn(t)
|
||||
seedRepo(t, nc, "team/app")
|
||||
body := `{"repositoryName":"team/app","policyText":` + strconvQuote("{}"+strings.Repeat(" ", 10240)) + `}`
|
||||
_, err := SetRepositoryPolicy(context.Background(), nc, policyTestAccount, []byte(body))
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorECRInvalidParameter, code)
|
||||
assert.Equal(t, "Invalid parameter at 'policyText' failed to satisfy constraint: 'Member must have length less than or equal to 10240'", message)
|
||||
}
|
||||
|
||||
// strconvQuote JSON-quotes a string for inline test bodies.
|
||||
func strconvQuote(s string) string {
|
||||
b, _ := json.Marshal(s)
|
||||
|
||||
@@ -3,6 +3,7 @@ package awsapi
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/ecr"
|
||||
@@ -13,6 +14,10 @@ import (
|
||||
const (
|
||||
minRepositoryNameLength = 2
|
||||
maxRepositoryNameLength = 256
|
||||
|
||||
minLifecyclePolicyTextLength = 100
|
||||
maxLifecyclePolicyTextLength = 30720
|
||||
maxRepositoryPolicyTextLength = 10240
|
||||
)
|
||||
|
||||
// ValidateRepositoryName returns InvalidParameterException naming the first
|
||||
@@ -47,6 +52,18 @@ func MaxItemsError(field string, limit int) error {
|
||||
return ConstraintError(field, fmt.Sprintf("Member must have length less than or equal to %d", limit))
|
||||
}
|
||||
|
||||
// ValidateTextLength refuses text whose length in characters falls outside
|
||||
// [minLen, maxLen], the bounds of field's model shape.
|
||||
func ValidateTextLength(field, text string, minLen, maxLen int) error {
|
||||
switch n := utf8.RuneCountInString(text); {
|
||||
case n < minLen:
|
||||
return ConstraintError(field, fmt.Sprintf("Member must have length greater than or equal to %d", minLen))
|
||||
case n > maxLen:
|
||||
return ConstraintError(field, fmt.Sprintf("Member must have length less than or equal to %d", maxLen))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ConstraintError refuses field for failing constraint.
|
||||
func ConstraintError(field, constraint string) error {
|
||||
return awserrors.Errorf(awserrors.ErrorECRInvalidParameter,
|
||||
|
||||
@@ -35,7 +35,7 @@ A rejection that succeeds is always a finding. A refusal counts as a pass only w
|
||||
|
||||
Request findings and undeclared errors block only for services listed under `promotedRequestServices` in `conformance-promoted-services.json`, which starts empty; response promotion does not carry over.
|
||||
|
||||
The 2026-09-29 baseline sent 4047 requests across 371 operations: 980 pass, 506 findings, 96 undeclared errors, 2465 inconclusive. None of it had been checked against AWS, so each finding below is a lead to verify before any fix. Since then, EC2 range-checks `MaxResults` on six `Describe*` calls and refuses a tag resource type the operation cannot tag, as AWS answers both, which clears 12 EC2 findings. IAM tag and untag calls now refuse a tag with no value or outside the model's key and value character sets, count tag lengths in characters rather than bytes, treat keys that differ only in case as one key on users and roles but as two elsewhere, and refuse the reserved `aws:` prefix, each as AWS answered it. IAM also checks tags on every create, `PathPrefix` on every list call (and `ListAttached*Policies` now filters by it), the `Scope` and `PolicyUsageFilter` enums, role and policy `Description` length, role `MaxSessionDuration` against 3600–43200, OIDC client ID and thumbprint lengths, `ListEntitiesForPolicy` `PolicyArn` length and an empty `CreatePolicy` `Path`, and answers name and path length breaks with `ValidationError`, each verified against AWS. STS `GetSessionToken` refuses an out-of-range `DurationSeconds` rather than clamping it, as AWS answers `AssumeRole`'s, and both calls check `SerialNumber` and `TokenCode` against the model's length and pattern before refusing MFA. ECR `CreateRepository` defaults to AES256 only when `encryptionConfiguration` is absent, and refuses one that omits the `encryptionType` its model requires. ECR `CreateRepository` and `TagResource` refuse a tag with no `Value`, as the model requires it, while still accepting an empty one. ECS `DeleteCluster` refuses a request that omits `cluster` with `InvalidParameterException` instead of resolving it to the default cluster and deleting that, which clears one ECS finding; AWS's code for a missing required member is not verified, and `ClientException` is the other one the operation declares. Every finding is traced to its cause, and grouped into fix items, in the mulga plan `docs/development/archive/bugs/aws-integration-conformance-findings.md`.
|
||||
The 2026-09-29 baseline sent 4047 requests across 371 operations: 980 pass, 506 findings, 96 undeclared errors, 2465 inconclusive. None of it had been checked against AWS, so each finding below is a lead to verify before any fix. Since then, EC2 range-checks `MaxResults` on six `Describe*` calls and refuses a tag resource type the operation cannot tag, as AWS answers both, which clears 12 EC2 findings. IAM tag and untag calls now refuse a tag with no value or outside the model's key and value character sets, count tag lengths in characters rather than bytes, treat keys that differ only in case as one key on users and roles but as two elsewhere, and refuse the reserved `aws:` prefix, each as AWS answered it. IAM also checks tags on every create, `PathPrefix` on every list call (and `ListAttached*Policies` now filters by it), the `Scope` and `PolicyUsageFilter` enums, role and policy `Description` length, role `MaxSessionDuration` against 3600–43200, OIDC client ID and thumbprint lengths, `ListEntitiesForPolicy` `PolicyArn` length and an empty `CreatePolicy` `Path`, and answers name and path length breaks with `ValidationError`, each verified against AWS. STS `GetSessionToken` refuses an out-of-range `DurationSeconds` rather than clamping it, as AWS answers `AssumeRole`'s, and both calls check `SerialNumber` and `TokenCode` against the model's length and pattern before refusing MFA. ECR `CreateRepository` defaults to AES256 only when `encryptionConfiguration` is absent, and refuses one that omits the `encryptionType` its model requires. ECR `CreateRepository` and `TagResource` refuse a tag with no `Value`, as the model requires it, while still accepting an empty one. ECS `DeleteCluster` refuses a request that omits `cluster` with `InvalidParameterException` instead of resolving it to the default cluster and deleting that, which clears one ECS finding; AWS's code for a missing required member is not verified, and `ClientException` is the other one the operation declares. ECR `PutLifecyclePolicy` now refuses an omitted policy text, and it, `SetRepositoryPolicy` and `StartLifecyclePolicyPreview` refuse one outside the model's length before parsing it, so a parseable document outside the limits is no longer accepted; this is checked against the model only, not against AWS. Every finding is traced to its cause, and grouped into fix items, in the mulga plan `docs/development/archive/bugs/aws-integration-conformance-findings.md`.
|
||||
|
||||
| Finding | Count | Notes |
|
||||
|---|---:|---|
|
||||
|
||||
Reference in New Issue
Block a user