amadan

carlos / platform Public

schedule: ReadDeclaration guards its IDs; direct checkIDs test

ReadDeclaration was missing the checkIDs guard its sibling readers
(WriteStatus, ListStatus) already carry, so a malformed acct/app
reached the store instead of being refused up front. Add the guard
and a direct test of checkIDs plus one pinning ReadDeclaration's
refusal.
Paul Campbell pushed by paul@keymail.dev c0aa3ac48353eb32a7be22f2aa03176f6024887b parent 797cbb8
2 files changed, +41 −0
  • internal/schedule/declaration.go +3 −0
  • internal/schedule/declaration_test.go +38 −0
diff --git a/internal/schedule/declaration.go b/internal/schedule/declaration.go
index fd87938ef..fcd24423c 100644
--- a/internal/schedule/declaration.go
+++ b/internal/schedule/declaration.go
@@ -138,6 +138,9 @@ func StatusKey(acct, app, host string) string { return StatusPrefix(acct, app) +
// ReadDeclaration: absent is the empty declaration with etag "" — legal
// forever, like restart-gen.
func ReadDeclaration(ctx context.Context, s release.Store, acct, app string) (Declaration, string, error) {
+ if err := checkIDs(acct, app); err != nil {
+ return Declaration{}, "", err
+ }
raw, etag, err := s.Get(ctx, DeclarationKey(acct, app))
if errors.Is(err, os.ErrNotExist) {
return Declaration{Schema: 1}, "", nil
diff --git a/internal/schedule/declaration_test.go b/internal/schedule/declaration_test.go
index 5734179d2..fcb679efe 100644
--- a/internal/schedule/declaration_test.go
+++ b/internal/schedule/declaration_test.go
@@ -97,6 +97,44 @@ func TestReadDeclarationAbsentIsEmpty(t *testing.T) {
}
}
+// ReadDeclaration takes an acct/app straight off a URL path in the console
+// door; it must refuse a malformed one itself rather than build a bucket
+// key from it and let the store report a confusing not-found.
+func TestReadDeclarationRejectsBadIDs(t *testing.T) {
+ ctx := context.Background()
+ st := release.NewDirStore(t.TempDir())
+ for _, tc := range []struct{ acct, app string }{
+ {"", "app"},
+ {"bab", ""},
+ {"BAB", "app"},
+ {"bab", "App"},
+ {"bab/../x", "app"},
+ } {
+ if _, _, err := ReadDeclaration(ctx, st, tc.acct, tc.app); err == nil {
+ t.Errorf("ReadDeclaration(%q, %q) = nil error, want a refusal", tc.acct, tc.app)
+ }
+ }
+}
+
+func TestCheckIDs(t *testing.T) {
+ if err := checkIDs("bab", "app"); err != nil {
+ t.Fatalf("checkIDs(valid) = %v", err)
+ }
+ for _, tc := range []struct{ acct, app string }{
+ {"", "app"},
+ {"bab", ""},
+ {"BAB", "app"},
+ {"bab", "App"},
+ {"b", "app"}, // too short for acctRe
+ {"bab", "-app"}, // appRe wants a leading letter
+ {"bab", "app/../etc"},
+ } {
+ if err := checkIDs(tc.acct, tc.app); err == nil {
+ t.Errorf("checkIDs(%q, %q) = nil, want a refusal", tc.acct, tc.app)
+ }
+ }
+}
+
func TestTouchRunMovesTheEtag(t *testing.T) {
ctx := context.Background()
st := release.NewDirStore(t.TempDir())