amadan

carlos / contra Public

web/8: compare-and-swap on item writes

The first of the two prerequisites #23 named, and the one that is a bug
rather than a hardening. store.UpdateItem was

    WHERE id = ? AND vault_id = ?

with no version precondition, writing whatever version the client sent.
With one device that is invisible. With two — or two members of a shared
vault, which is the point of this branch — it is silent, total data loss:
both open v3, both edit, both write v4, and the second erases the first
with nothing anywhere recording that an edit existed.

Now `AND version = ?` against the version the client says it is replacing,
with zero rows affected returning 409 carrying the CURRENT version, so the
loser can refetch and merge instead of guessing whether to resend the same
bytes. That is Eleven's putBlob contract applied per item rather than per
user, which is what scopes a conflict to one item instead of a vault.

The client raises StaleItemError rather than retrying, and that is
deliberate: the edit in hand was composed against bytes that are no longer
current, so resending it is precisely the silent erase this exists to stop.

Also refuses a version that is not exactly expected+1. The client picks the
bytes; it does not pick the numbering, and accepting a repeat would let a
client reuse a version, which undercuts what binding the version into the
AAD is for.

While here, corrected an overclaim in the comment this replaces. The
version in the AAD stops RELABELLING — a server cannot serve v3 as v7 — and
has never stopped rollback: serving the whole older (version, nonce,
ciphertext) triple consistently yields an item that opens, because it is a
genuinely older authentic item. That limitation is now in DESIGN.md's
residue section and named here rather than glossed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev a974578de1f73577c0052a56aaaf3603be677ffb parent 12b61f4
7 files changed, +320 −28
  • internal/contra/static/crypto/items.js +33 −6
  • internal/contra/store/store.go +68 −8
  • internal/contra/vault/vault.go +35 −7
  • internal/contratest/cas_test.go +174 −0
  • internal/contratest/greps_test.go +1 −1
  • internal/contratest/jsmodules_test.go +1 −1
  • internal/contratest/vault_test.go +8 −5

CI passed — run details

CI log 
=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./...
?   	github.com/carlosframework/contra/cmd/contra	[no test files]
?   	github.com/carlosframework/contra/internal/contra	[no test files]
?   	github.com/carlosframework/contra/internal/contra/auth	[no test files]
ok  	github.com/carlosframework/contra/internal/contra/crypto	(cached)
?   	github.com/carlosframework/contra/internal/contra/icons	[no test files]
?   	github.com/carlosframework/contra/internal/contra/mail	[no test files]
?   	github.com/carlosframework/contra/internal/contra/model	[no test files]
?   	github.com/carlosframework/contra/internal/contra/store	[no test files]
?   	github.com/carlosframework/contra/internal/contra/vault	[no test files]
ok  	github.com/carlosframework/contra/internal/contratest	11.372s
=== js-test ===
node --test test/crypto.test.mjs test/ecies.test.mjs test/items.test.mjs
TAP version 13
# Subtest: HKDF: every contra/v1 label
ok 1 - HKDF: every contra/v1 label
  ---
  duration_ms: 10.253103
  type: 'test'
  ...
# Subtest: KCV
ok 2 - KCV
  ---
  duration_ms: 2.238834
  type: 'test'
  ...
# Subtest: P-256 from a seed, including the rejection branch
ok 3 - P-256 from a seed, including the rejection branch
  ---
  duration_ms: 12.795444
  type: 'test'
  ...
# Subtest: signed challenge round-trips against the derived public key
ok 4 - signed challenge round-trips against the derived public key
  ---
  duration_ms: 5.019711
  type: 'test'
  ...
# Subtest: Crockford: alphabet, encoding, normalisation, strict decoding
ok 5 - Crockford: alphabet, encoding, normalisation, strict decoding
  ---
  duration_ms: 0.981852
  type: 'test'
  ...
# Subtest: the printed code, and the O/I/L confusions it has to survive
ok 6 - the printed code, and the O/I/L confusions it has to survive
  ---
  duration_ms: 2.164911
  type: 'test'
  ...
# Subtest: repair recovers one substituted character
ok 7 - repair recovers one substituted character
  ---
  duration_ms: 47.797273
  type: 'test'
  ...
# Subtest: AEAD: the AAD binding and every wrap
ok 8 - AEAD: the AAD binding and every wrap
  ---
  duration_ms: 2.594207
  type: 'test'
  ...
# Subtest: AEAD: the item, scoped to its vault
ok 9 - AEAD: the item, scoped to its vault
  ---
  duration_ms: 1.120229
  type: 'test'
  ...
# Subtest: a transplanted ciphertext does not open
ok 10 - a transplanted ciphertext does not open
  ---
  duration_ms: 1.906402
  type: 'test'
  ...
# Subtest: padding quantises into 256-byte buckets and round-trips
ok 11 - padding quantises into 256-byte buckets and round-trips
  ---
  duration_ms: 0.924821
  type: 'test'
  ...
# Subtest: the KDF parameter floor refuses what a malicious server would offer
ok 12 - the KDF parameter floor refuses what a malicious server would offer
  ---
  duration_ms: 0.590362
  type: 'test'
  ...
# Subtest: Argon2id matches the published PHC reference vector
ok 13 - Argon2id matches the published PHC reference vector
  ---
  duration_ms: 1806.986643
  type: 'test'
  ...
# Subtest: Argon2id at the corners the 64 MiB case does not reach
ok 14 - Argon2id at the corners the 64 MiB case does not reach
  ---
  duration_ms: 214.881416
  type: 'test'
  ...
# Subtest: Argon2id matches the golden vector at the parameters DESIGN.md pins
ok 15 - Argon2id matches the golden vector at the parameters DESIGN.md pins
  ---
  duration_ms: 2582.464072
  type: 'test'
  ...
# Subtest: the account chain, end to end
ok 16 - the account chain, end to end
  ---
  duration_ms: 2584.289566
  type: 'test'
  ...
# Subtest: handoff: both keypairs derive from their published seeds
ok 17 - handoff: both keypairs derive from their published seeds
  ---
  duration_ms: 16.533001
  type: 'test'
  ...
# Subtest: handoff: ECDH agrees from both sides
ok 18 - handoff: ECDH agrees from both sides
  ---
  duration_ms: 8.230885
  type: 'test'
  ...
# Subtest: handoff: the derived key, the AAD and the sealed bytes
ok 19 - handoff: the derived key, the AAD and the sealed bytes
  ---
  duration_ms: 2.435548
  type: 'test'
  ...
# Subtest: handoff: an envelope opens for its device and for nobody else
ok 20 - handoff: an envelope opens for its device and for nobody else
  ---
  duration_ms: 16.758874
  type: 'test'
  ...
# Subtest: handoff: the Go and JS halves seal for each other
ok 21 - handoff: the Go and JS halves seal for each other
  ---
  duration_ms: 8.194939
  type: 'test'
  ...
# Subtest: handoff: a truncated envelope is refused before any key work
ok 22 - handoff: a truncated envelope is refused before any key work
  ---
  duration_ms: 0.617768
  type: 'test'
  ...
# Subtest: an item round-trips through create, list and update
ok 23 - an item round-trips through create, list and update
  ---
  duration_ms: 11.412065
  type: 'test'
  ...
# Subtest: nothing readable reaches the network
ok 24 - nothing readable reaches the network
  ---
  duration_ms: 2.52205
  type: 'test'
  ...
# Subtest: item ciphertexts are quantised into 256-byte buckets
ok 25 - item ciphertexts are quantised into 256-byte buckets
  ---
  duration_ms: 1.689272
  type: 'test'
  ...
# Subtest: an item does not open in another slot
ok 26 - an item does not open in another slot
  ---
  duration_ms: 2.866805
  type: 'test'
  ...
# Subtest: every field is present whether or not it was filled in
ok 27 - every field is present whether or not it was filled in
  ---
  duration_ms: 1.293382
  type: 'test'
  ...
# Subtest: the Account Key cache is storage handed in, not reached for
ok 28 - the Account Key cache is storage handed in, not reached for
  ---
  duration_ms: 0.352311
  type: 'test'
  ...
1..28
# tests 28
# suites 0
# pass 28
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 7346.853788
=== migration-check ===
go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check
rastrillo migration check: models and migrations agree
diff --git a/internal/contra/static/crypto/items.js b/internal/contra/static/crypto/items.js
index da66539..d68e96f 100644
--- a/internal/contra/static/crypto/items.js
+++ b/internal/contra/static/crypto/items.js
@@ -41,6 +41,21 @@ export async function openItem(vaultId, kVault, itemId, version, sealed) {
return body(JSON.parse(new TextDecoder().decode(plaintext)));
}
+// StaleItemError is a lost compare-and-swap: somebody wrote this item between
+// this client reading it and writing it back.
+//
+// Its own type because the caller's response is specific and is not "retry":
+// the edit in hand was composed against bytes that are no longer current, so
+// resending it is exactly the silent erase the CAS exists to stop. Refetch,
+// show both, let the person choose.
+export class StaleItemError extends Error {
+ constructor(item, cause) {
+ super("contra: this item changed somewhere else while you were editing it");
+ this.item = item;
+ this.cause = cause;
+ }
+}
+
// Items is the vault's item list as the console works with it: opened once on
// load, kept in memory, never written back to disk unsealed.
export function items(net, vault) {
@@ -75,15 +90,27 @@ export function items(net, vault) {
// Editing bumps the version, and the version is in the AAD — so a server
// that served an old ciphertext back under a new number would produce a
// blob that does not open, rather than one that quietly reverts an edit.
+ //
+ // expected_version is the compare-and-swap: it says which version this
+ // edit replaces, and the server refuses with 409 if somebody else got
+ // there first. Without it the second of two writers silently erases the
+ // first, which is invisible on one device and is data loss the moment
+ // there are two — or two members of a shared vault.
async update(item, fields) {
const version = item.version + 1;
const sealed = await sealItem(vault.id, vault.kVault, item.id, version, fields);
- await net.put("/api/items", {
- vault_id_hex: vault.idHex,
- item_id_hex: item.idHex,
- version,
- ...sealed,
- });
+ try {
+ await net.put("/api/items", {
+ vault_id_hex: vault.idHex,
+ item_id_hex: item.idHex,
+ expected_version: item.version,
+ version,
+ ...sealed,
+ });
+ } catch (err) {
+ if (err.status === 409) throw new StaleItemError(item, err);
+ throw err;
+ }
return { ...item, version, fields: body(fields) };
},
diff --git a/internal/contra/store/store.go b/internal/contra/store/store.go
index b95295a..a5005db 100644
--- a/internal/contra/store/store.go
+++ b/internal/contra/store/store.go
@@ -42,6 +42,15 @@ const FormatVersion = 1
var (
ErrNotAligned = errors.New("store: item ciphertext is not quantised into 256-byte buckets")
ErrShortField = errors.New("store: a required sealed field is empty or the wrong length")
+
+ // ErrStale is a lost compare-and-swap: somebody wrote this item between
+ // the caller reading it and the caller writing it. The API answers 409
+ // with the current version so the client can merge rather than guess.
+ ErrStale = errors.New("store: this item changed since you read it")
+
+ // ErrBadVersion is a client numbering its own write wrongly — a gap or a
+ // repeat rather than expected+1.
+ ErrBadVersion = errors.New("store: an item version must be exactly one more than the one it replaces")
)
// Store writes the sealed tables. It holds a *gorm.DB and no keys, because
@@ -338,28 +347,79 @@ func (s *Store) PutItem(ctx context.Context, accountID, vaultID, itemID []byte,
return it, s.db.WithContext(ctx).Create(it).Error
}
-// UpdateItem replaces an item's sealed body and bumps its version. The
-// version is bound into the client's AAD, so a server that served an old
-// ciphertext back under a new version number would produce a blob that does
-// not open.
+// UpdateItem replaces an item's sealed body and bumps its version.
+//
+// # Compare-and-swap, and why it is not optional
+//
+// The WHERE carries `version = ?` — the version the client believes it is
+// replacing — and zero rows affected is ErrStale, which the API turns into a
+// 409 carrying the current version.
+//
+// Without that precondition this wrote whatever version the client sent over
+// whatever was there. With one device that is invisible. With two, or with
+// two members of a shared vault, it is silent data loss: both open version 3,
+// both edit, both write version 4, and the second simply erases the first
+// with nothing anywhere recording that an edit existed. It also let a client
+// reuse a version number, which undercuts what binding the version into the
+// AAD is for.
+//
+// The version in the AAD stops RELABELLING — a server cannot serve ciphertext
+// v3 as v7. It has never stopped rollback, and does not claim to: serving the
+// whole older (version, nonce, ciphertext) triple consistently yields an item
+// that opens, because it is a genuinely older authentic item. That is in
+// DESIGN.md's residue section, not papered over here.
+//
+// This is Eleven's putBlob contract applied per item rather than per user,
+// which is the shape that lets a conflict scope to one item instead of a
+// whole vault.
func (s *Store) UpdateItem(ctx context.Context, accountID, vaultID, itemID []byte,
- version uint64, sealed Sealed) error {
+ expected, version uint64, sealed Sealed) error {
if !sealed.valid() {
return ErrShortField
}
if !crypto.IsBucketAligned(sealed.Ciphertext) {
return ErrNotAligned
}
+ if version != expected+1 {
+ // The client decides the bytes; it does not get to decide the
+ // numbering. A gap or a repeat here means a confused writer.
+ return ErrBadVersion
+ }
if _, err := s.ownedVault(ctx, accountID, vaultID); err != nil {
return err
}
- return s.db.WithContext(ctx).Model(&model.Item{}).
- Where("id = ? AND vault_id = ?", itemID, vaultID).
+ res := s.db.WithContext(ctx).Model(&model.Item{}).
+ Where("id = ? AND vault_id = ? AND version = ?", itemID, vaultID, expected).
Select("Version", "Nonce", "Ciphertext", "UpdatedAt").
Updates(map[string]any{
"Version": version,
"Nonce": sealed.Nonce,
"Ciphertext": sealed.Ciphertext,
"UpdatedAt": time.Now().UTC(),
- }).Error
+ })
+ if res.Error != nil {
+ return res.Error
+ }
+ if res.RowsAffected == 0 {
+ // Either the item is gone or somebody else wrote first. Both are
+ // "refetch and try again", and telling them apart across a vault the
+ // caller can already list is not worth a second query.
+ return ErrStale
+ }
+ return nil
+}
+
+// ItemVersion reports the version currently stored, so a 409 can tell the
+// client what it has to merge against rather than only that it lost.
+func (s *Store) ItemVersion(ctx context.Context, accountID, vaultID, itemID []byte) (uint64, error) {
+ if _, err := s.ownedVault(ctx, accountID, vaultID); err != nil {
+ return 0, err
+ }
+ var it model.Item
+ err := s.db.WithContext(ctx).Select("version").
+ Where("id = ? AND vault_id = ?", itemID, vaultID).Take(&it).Error
+ if err != nil {
+ return 0, err
+ }
+ return it.Version, nil
}
diff --git a/internal/contra/vault/vault.go b/internal/contra/vault/vault.go
index 1a8a3fd..041f017 100644
--- a/internal/contra/vault/vault.go
+++ b/internal/contra/vault/vault.go
@@ -202,6 +202,12 @@ type putItemRequest struct {
VaultIDHex string `json:"vault_id_hex"`
ItemIDHex string `json:"item_id_hex"`
Version uint64 `json:"version"`
+
+ // ExpectedVersion is the version this write believes it is replacing —
+ // the compare-and-swap precondition. Ignored on create, where there is
+ // nothing to replace.
+ ExpectedVersion uint64 `json:"expected_version"`
+
sealedJSON
}
@@ -239,9 +245,14 @@ func (v *Vault) CreateItem(w http.ResponseWriter, r *http.Request) {
})
}
-// UpdateItem replaces an item's sealed body and bumps its version. The
-// version is in the client's AAD, so a server serving an old ciphertext back
-// under a new number produces a blob that does not open.
+// UpdateItem replaces an item's sealed body and bumps its version.
+//
+// A compare-and-swap: the client says which version it is replacing, and a
+// write that lost the race gets 409 with the version that actually won, so it
+// can refetch and merge rather than silently erase somebody. That matters
+// from the moment a second device or a second member exists — before this,
+// two writers who both opened v3 both wrote v4 and the loser vanished with
+// nothing recording that it had ever been there.
func (v *Vault) UpdateItem(w http.ResponseWriter, r *http.Request) {
acct, ok := accountID(r)
if !ok {
@@ -259,15 +270,32 @@ func (v *Vault) UpdateItem(w http.ResponseWriter, r *http.Request) {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "bad item"})
return
}
- if err := v.store.UpdateItem(r.Context(), acct, vaultID, itemID, req.Version, sealed); err != nil {
- if errors.Is(err, gorm.ErrRecordNotFound) {
+
+ err := v.store.UpdateItem(r.Context(), acct, vaultID, itemID,
+ req.ExpectedVersion, req.Version, sealed)
+ switch {
+ case err == nil:
+ writeJSON(w, http.StatusOK, map[string]string{"item_id_hex": req.ItemIDHex})
+ case errors.Is(err, gorm.ErrRecordNotFound):
+ notFound(w)
+ case errors.Is(err, store.ErrStale):
+ // Tell them what to merge against. A bare 409 leaves the client
+ // guessing whether to refetch or to retry the same bytes.
+ current, lookupErr := v.store.ItemVersion(r.Context(), acct, vaultID, itemID)
+ if lookupErr != nil {
+ // It was deleted between the failed write and this read, which is
+ // the other reason the CAS missed.
notFound(w)
return
}
+ writeJSON(w, http.StatusConflict, map[string]any{
+ "error": "this item changed since you read it",
+ "item_id_hex": req.ItemIDHex,
+ "current_version": current,
+ })
+ default:
writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()})
- return
}
- writeJSON(w, http.StatusOK, map[string]string{"item_id_hex": req.ItemIDHex})
}
type deleteItemRequest struct {
diff --git a/internal/contratest/cas_test.go b/internal/contratest/cas_test.go
new file mode 100644
index 0000000..5903fc2
--- /dev/null
+++ b/internal/contratest/cas_test.go
@@ -0,0 +1,174 @@
+package contratest
+
+import (
+ "encoding/hex"
+ "encoding/json"
+ "net/http"
+ "testing"
+
+ "github.com/carlosframework/contra/internal/contra/crypto"
+)
+
+// Compare-and-swap on item writes.
+//
+// The failure this prevents is silent and total: two writers both open
+// version 3, both edit, both write version 4, and the second erases the first
+// with nothing anywhere recording that an edit ever existed. On one device it
+// cannot happen. With a second device — or a second member of a shared vault
+// — it is the ordinary case, which is why this landed before web/8 rather
+// than with it.
+
+func TestTwoWritersDoNotSilentlyOverwriteEachOther(t *testing.T) {
+ h := newApp(t)
+ p := provision(t, h, "owner@example.invalid", "a generated test passphrase 8f3a1c")
+
+ itemID := newItemID(t)
+ if rec := as(t, h, p.cookie, http.MethodPost, "/api/items",
+ sealItem(t, p, itemID, 1, `{"title":"Original"}`)); rec.Code != http.StatusCreated {
+ t.Fatalf("create: status %d", rec.Code)
+ }
+
+ // Two devices have both read version 1 and both composed an edit against
+ // it. The first one home wins.
+ first := sealItem(t, p, itemID, 2, `{"title":"Edited by the first device"}`)
+ second := sealItem(t, p, itemID, 2, `{"title":"Edited by the second device"}`)
+
+ if rec := as(t, h, p.cookie, http.MethodPut, "/api/items", first); rec.Code != http.StatusOK {
+ t.Fatalf("first write: status %d, body %s", rec.Code, rec.Body.String())
+ }
+
+ rec := as(t, h, p.cookie, http.MethodPut, "/api/items", second)
+ if rec.Code != http.StatusConflict {
+ t.Fatalf("second write: status %d, want 409 — it overwrote the first", rec.Code)
+ }
+
+ // And the 409 says what to merge against, so the loser can refetch rather
+ // than guess whether to resend the same bytes.
+ var conflict struct {
+ CurrentVersion uint64 `json:"current_version"`
+ ItemIDHex string `json:"item_id_hex"`
+ }
+ if err := json.Unmarshal(rec.Body.Bytes(), &conflict); err != nil {
+ t.Fatalf("409 body: %v", err)
+ }
+ if conflict.CurrentVersion != 2 {
+ t.Errorf("409 reported current version %d, want 2", conflict.CurrentVersion)
+ }
+ if conflict.ItemIDHex != hex.EncodeToString(itemID) {
+ t.Errorf("409 named the wrong item")
+ }
+
+ // The stored item is the FIRST device's edit, intact.
+ body := openOnlyItem(t, h, p)
+ if body != `{"title":"Edited by the first device"}` {
+ t.Errorf("the losing write was not discarded\n got %s", body)
+ }
+}
+
+// Having lost, the client refetches and writes against what it found. That
+// has to succeed, or the conflict is a dead end rather than a retry.
+func TestALostWriteSucceedsAfterRefetching(t *testing.T) {
+ h := newApp(t)
+ p := provision(t, h, "owner@example.invalid", "a generated test passphrase 8f3a1c")
+
+ itemID := newItemID(t)
+ as(t, h, p.cookie, http.MethodPost, "/api/items",
+ sealItem(t, p, itemID, 1, `{"title":"Original"}`))
+ as(t, h, p.cookie, http.MethodPut, "/api/items",
+ sealItem(t, p, itemID, 2, `{"title":"Theirs"}`))
+
+ // Rejected against the stale version, accepted against the current one.
+ if rec := as(t, h, p.cookie, http.MethodPut, "/api/items",
+ sealItem(t, p, itemID, 2, `{"title":"Mine"}`)); rec.Code != http.StatusConflict {
+ t.Fatalf("stale write: status %d, want 409", rec.Code)
+ }
+ if rec := as(t, h, p.cookie, http.MethodPut, "/api/items",
+ sealItem(t, p, itemID, 3, `{"title":"Mine, merged"}`)); rec.Code != http.StatusOK {
+ t.Fatalf("merged write: status %d, want 200", rec.Code)
+ }
+ if body := openOnlyItem(t, h, p); body != `{"title":"Mine, merged"}` {
+ t.Errorf("the merged write did not land\n got %s", body)
+ }
+}
+
+// The client picks the bytes; it does not get to pick the numbering. A gap or
+// a repeat means a confused writer, and accepting one would let a client
+// reuse a version — which undercuts what binding the version into the AAD is
+// for.
+func TestItemVersionsMustBeExactlyOneApart(t *testing.T) {
+ h := newApp(t)
+ p := provision(t, h, "owner@example.invalid", "a generated test passphrase 8f3a1c")
+
+ itemID := newItemID(t)
+ as(t, h, p.cookie, http.MethodPost, "/api/items",
+ sealItem(t, p, itemID, 1, `{"title":"Original"}`))
+
+ for _, c := range []struct {
+ what string
+ expected uint64
+ version uint64
+ }{
+ {"a gap", 1, 5},
+ {"no bump at all", 1, 1},
+ {"going backwards", 1, 0},
+ } {
+ body := sealItem(t, p, itemID, c.version, `{"title":"x"}`)
+ body["expected_version"] = c.expected
+ if rec := as(t, h, p.cookie, http.MethodPut, "/api/items", body); rec.Code == http.StatusOK {
+ t.Errorf("%s was accepted", c.what)
+ }
+ }
+}
+
+// A vault that is not yours answers 404 on the CAS path too. Worth its own
+// case because the precondition added a second way to miss, and "somebody
+// else wrote first" must never be reported for a row the caller cannot see.
+func TestCASDoesNotLeakAnotherAccountsItem(t *testing.T) {
+ h := newApp(t)
+ mine := provision(t, h, "owner@example.invalid", "a generated test passphrase 8f3a1c")
+ theirs := provision(t, h, "other@example.invalid", "a different generated one 4d2e9b")
+
+ itemID := newItemID(t)
+ as(t, h, mine.cookie, http.MethodPost, "/api/items",
+ sealItem(t, mine, itemID, 1, `{"title":"Mine"}`))
+
+ body := sealItem(t, mine, itemID, 2, `{"title":"Theirs now"}`)
+ if rec := as(t, h, theirs.cookie, http.MethodPut, "/api/items", body); rec.Code != http.StatusNotFound {
+ t.Errorf("another account's update: status %d, want 404", rec.Code)
+ }
+}
+
+// openOnlyItem reads the vault's single item back and returns its plaintext,
+// so a test asserts on what a client would actually see rather than on a
+// status code.
+func openOnlyItem(t *testing.T, h http.Handler, p provisioned) string {
+ t.Helper()
+ var listed struct {
+ Items []struct {
+ ItemIDHex string `json:"item_id_hex"`
+ Version uint64 `json:"version"`
+ NonceHex string `json:"nonce_hex"`
+ CiphertextHex string `json:"ciphertext_hex"`
+ } `json:"items"`
+ }
+ rec := as(t, h, p.cookie, http.MethodGet,
+ "/api/items?vault="+hex.EncodeToString(p.vaultID), nil)
+ decode(t, rec, &listed)
+ if len(listed.Items) != 1 {
+ t.Fatalf("expected exactly one item, got %d", len(listed.Items))
+ }
+ row := listed.Items[0]
+ id, _ := hex.DecodeString(row.ItemIDHex)
+ nonce, _ := hex.DecodeString(row.NonceHex)
+ ct, _ := hex.DecodeString(row.CiphertextHex)
+ plain, err := crypto.Open(p.kVault, nonce,
+ crypto.AAD(p.vaultID, crypto.ObjectItem, id, row.Version), ct)
+ if err != nil {
+ t.Fatalf("the stored item does not open: %v", err)
+ }
+ unpadded, err := crypto.Unpad(plain)
+ if err != nil {
+ t.Fatalf("unpad: %v", err)
+ }
+ return string(unpadded)
+}
diff --git a/internal/contratest/greps_test.go b/internal/contratest/greps_test.go
index daf8dd1..e8b1d5f 100644
--- a/internal/contratest/greps_test.go
+++ b/internal/contratest/greps_test.go
@@ -266,7 +266,7 @@ func writeEverything(t *testing.T, s *store.Store) secrets {
if err != nil {
t.Fatalf("Seal(edit): %v", err)
}
- if err := s.UpdateItem(ctx, acct.ID, vault.ID, item.ID, 2,
+ if err := s.UpdateItem(ctx, acct.ID, vault.ID, item.ID, 1, 2,
store.Sealed{Nonce: editNonce, Ciphertext: editCT}); err != nil {
t.Fatalf("UpdateItem: %v", err)
}
diff --git a/internal/contratest/jsmodules_test.go b/internal/contratest/jsmodules_test.go
index ffe7ed5..726801d 100644
--- a/internal/contratest/jsmodules_test.go
+++ b/internal/contratest/jsmodules_test.go
@@ -32,7 +32,7 @@ var jsHighWater = map[string]int{
"bytes.js": 61,
"code.js": 165,
"ecies.js": 132,
- "items.js": 97,
+ "items.js": 124,
"kdf.js": 129,
"link.js": 131,
"kit.js": 203,
diff --git a/internal/contratest/vault_test.go b/internal/contratest/vault_test.go
index 792efd5..d99e86c 100644
--- a/internal/contratest/vault_test.go
+++ b/internal/contratest/vault_test.go
@@ -131,11 +131,14 @@ func sealItem(t *testing.T, p provisioned, itemID []byte, version uint64, body s
t.Fatal(err)
}
return map[string]any{
- "vault_id_hex": hex.EncodeToString(p.vaultID),
- "item_id_hex": hex.EncodeToString(itemID),
- "version": version,
- "nonce_hex": hex.EncodeToString(nonce),
- "ciphertext_hex": hex.EncodeToString(ct),
+ "vault_id_hex": hex.EncodeToString(p.vaultID),
+ "item_id_hex": hex.EncodeToString(itemID),
+ "version": version,
+ // The compare-and-swap precondition: what this write replaces.
+ // Ignored on create, where there is nothing to replace.
+ "expected_version": version - 1,
+ "nonce_hex": hex.EncodeToString(nonce),
+ "ciphertext_hex": hex.EncodeToString(ct),
}
}