fix(oauth): refuse keyring indexes over the reader key cap on write

writeKeyIndex already refused over-cap chunk counts, but a large set of
short keys can stay under the chunk limit while exceeding
maxKeyringIndexKeys. Cap the key count before publishing so Save cannot
persist an index that readKeyIndex then rejects. Cover the write-side
path and multi-chunk accumulation on the reader.

Author: euxaristia

internal/oauth/store.go

@@ -860,10 +860,15 @@ func (b keyringBlob) readKeyIndex() ([]string, bool, int, error) {
 // only after the header stops referencing them (best-effort: an unreferenced
 // chunk is never read).
 func (b keyringBlob) writeKeyIndex(keys []string, priorChunks int) (int, error) {
+	// Refuse to publish an index the reader would reject: readKeyIndex caps both
+	// total keys and chunk count, and a header beyond either would make every
+	// later Load/Status/Save/Delete fail before it could recover. Check the key
+	// count before chunking so a large set of short keys that still fit under
+	// maxKeyringIndexChunks cannot strand the store unreadable.
+	if len(keys) > maxKeyringIndexKeys {
+		return 0, errKeyringIndexTooManyKeys(len(keys))
+	}
 	chunks := chunkIndexKeys(keys)
-	// Refuse to publish an index the reader would reject: readKeyIndex caps
-	// headers at maxKeyringIndexChunks, and a header beyond it would make
-	// every later Load/Status/Save/Delete fail before it could recover.
 	if len(chunks) > maxKeyringIndexChunks {
 		return 0, fmt.Errorf("oauth: keyring key index needs %d chunks, over the %d-chunk cap readers accept; too many stored credentials", len(chunks), maxKeyringIndexChunks)
 	}

internal/oauth/store_keyring_test.go

@@ -851,19 +851,48 @@ func TestStoreKeyringReadIndexRejectsOversizedKeyList(t *testing.T) {
 		t.Fatal(err)
 	}
 	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(header)
+	ckr.gets = 0
 	if _, _, _, err := blob.readKeyIndex(); err == nil {
 		t.Fatal("expected an oversized key list in a chunk-0 header to be rejected")
 	}
+	if ckr.gets != 1 {
+		t.Fatalf("readKeyIndex issued %d gets for an oversized header keys list; want header lookup only", ckr.gets)
+	}
 
 	// The pre-chunking bare-array format must be capped the same way.
 	legacyArray, err := json.Marshal(tooMany)
 	if err != nil {
 		t.Fatal(err)
 	}
 	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(legacyArray)
+	ckr.gets = 0
 	if _, _, _, err := blob.readKeyIndex(); err == nil {
 		t.Fatal("expected an oversized legacy-format key array to be rejected")
 	}
+	if ckr.gets != 1 {
+		t.Fatalf("readKeyIndex issued %d gets for an oversized legacy array; want header lookup only", ckr.gets)
+	}
+
+	// Accumulation across continuation chunks must hit the same total cap: a
+	// small header plus an oversized chunk-1 would otherwise fan out past the
+	// bound after the per-header check has already passed.
+	headerOK, err := json.Marshal(keyIndexHeader{Version: 1, Chunks: 2, Keys: []string{ProviderKey("seed")}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	chunk1, err := json.Marshal(tooMany)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ckr.data[keyringService+"/"+keyringIndexAccount] = base64.StdEncoding.EncodeToString(headerOK)
+	ckr.data[keyringService+"/"+keyringIndexAccount+"-1"] = base64.StdEncoding.EncodeToString(chunk1)
+	ckr.gets = 0
+	if _, _, _, err := blob.readKeyIndex(); err == nil {
+		t.Fatal("expected an oversized key list accumulated across chunks to be rejected")
+	}
+	if ckr.gets != 2 {
+		t.Fatalf("readKeyIndex issued %d gets for a header+oversize-chunk; want header and chunk-1 only", ckr.gets)
+	}
 }
 
 // TestKeyringFallbackLockPathIsPerUser covers the fallback taken when no config
@@ -969,3 +998,23 @@ func TestStoreKeyringWriteIndexRejectsOverCapChunks(t *testing.T) {
 		t.Fatalf("over-cap index write must publish nothing, found %d entries", len(kr.data))
 	}
 }
+
+// TestStoreKeyringWriteIndexRejectsOverCapKeys: short keys can still fit under
+// the chunk-count cap while exceeding maxKeyringIndexKeys. writeKeyIndex must
+// refuse that set before publishing, matching the reader-side total key cap.
+func TestStoreKeyringWriteIndexRejectsOverCapKeys(t *testing.T) {
+	kr := newFakeKR()
+	b := keyringBlob{kr: kr, service: keyringService, legacyAccount: keyringLegacyAccount, indexAccount: keyringIndexAccount}
+	keys := make([]string, maxKeyringIndexKeys+1)
+	for i := range keys {
+		// Short keys pack densely into chunks so the chunk-count check alone
+		// would not catch this over-cap set.
+		keys[i] = fmt.Sprintf("p%d", i)
+	}
+	if _, err := b.writeKeyIndex(keys, 0); err == nil {
+		t.Fatal("writeKeyIndex published a key count readKeyIndex would refuse")
+	}
+	if len(kr.data) != 0 {
+		t.Fatalf("over-cap key write must publish nothing, found %d entries", len(kr.data))
+	}
+}