From 2c1bded6d53ed87f0ebbc2d24480654edac26ada Mon Sep 17 00:00:00 2001 From: Shane Jaroch Date: Tue, 18 Aug 2026 17:46:39 -0400 Subject: [PATCH 1/6] chore: support sharding/parallel runs, namespace scopes via `COMPLEMENT_CRYPTO_NAMESPACE` --- tests/main_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/main_test.go b/tests/main_test.go index 710856d..ef0b6bc 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -1,6 +1,7 @@ package tests import ( + "os" "testing" "github.com/matrix-org/complement-crypto/internal/cc" @@ -15,7 +16,16 @@ var ( // Main entry point when users run `go test`. Defined in https://pkg.go.dev/testing#hdr-Main func TestMain(m *testing.M) { instance = cc.NewInstance(config.NewComplementCryptoConfigFromEnvVars("./mitmproxy_addons")) - instance.TestMain(m, "crypto") + // The namespace prefixes every docker network/container this suite deploys + // (e.g. `complement_..hs1`). It must be unique per + // `go test` process so concurrent sharded runs get fully isolated + // homeservers instead of colliding on the same name. Defaults to `crypto` + // for a single (unsharded) run. + namespace := os.Getenv("COMPLEMENT_CRYPTO_NAMESPACE") + if namespace == "" { + namespace = "crypto" + } + instance.TestMain(m, namespace) } From ae357847cde496292c46a4c57b4ed5c9a997937c Mon Sep 17 00:00:00 2001 From: Shane Jaroch Date: Tue, 18 Aug 2026 19:32:04 -0400 Subject: [PATCH 2/6] tests: validate COMPLEMENT_CRYPTO_NAMESPACE before use An invalid namespace would flow into Docker container/network names and fail with a low-level Docker error. Reject characters outside [A-Za-z0-9_.-] with a clear message, preserving the 'crypto' default, and add coverage for rejected values. --- tests/main_test.go | 29 ++++++++++++++++++++--------- tests/namespace_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 tests/namespace_test.go diff --git a/tests/main_test.go b/tests/main_test.go index ef0b6bc..35dc2b3 100644 --- a/tests/main_test.go +++ b/tests/main_test.go @@ -16,19 +16,30 @@ var ( // Main entry point when users run `go test`. Defined in https://pkg.go.dev/testing#hdr-Main func TestMain(m *testing.M) { instance = cc.NewInstance(config.NewComplementCryptoConfigFromEnvVars("./mitmproxy_addons")) - // The namespace prefixes every docker network/container this suite deploys - // (e.g. `complement_..hs1`). It must be unique per - // `go test` process so concurrent sharded runs get fully isolated - // homeservers instead of colliding on the same name. Defaults to `crypto` - // for a single (unsharded) run. - namespace := os.Getenv("COMPLEMENT_CRYPTO_NAMESPACE") - if namespace == "" { - namespace = "crypto" - } + namespace := resolveNamespace(os.Getenv("COMPLEMENT_CRYPTO_NAMESPACE")) instance.TestMain(m, namespace) } +// resolveNamespace returns the namespace applied to every Docker network/container +// this suite deploys (e.g. `complement_..hs1`). It must be +// unique per `go test` process so concurrent sharded runs get fully isolated +// homeservers instead of colliding on the same name. Defaults to `crypto` for a +// single (unsharded) run. An empty value (or the default) is fine, but any value +// containing characters outside [A-Za-z0-9_.-] would produce invalid Docker names +// and fail with a low-level Docker error, so we reject it here with a clear message. +func resolveNamespace(raw string) string { + if raw == "" { + raw = "crypto" + } + for _, r := range raw { + if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '_' || r == '.' || r == '-') { + panic("COMPLEMENT_CRYPTO_NAMESPACE must contain only characters in [A-Za-z0-9_.-], got: " + raw) + } + } + return raw +} + // Instance returns the test instance. Guaranteed to be non-nil if called in a test, // because TestMain would have been called before the test runs. func Instance() *cc.Instance { diff --git a/tests/namespace_test.go b/tests/namespace_test.go new file mode 100644 index 0000000..3698161 --- /dev/null +++ b/tests/namespace_test.go @@ -0,0 +1,31 @@ +package tests + +import "testing" + +func TestResolveNamespace(t *testing.T) { + // acceptable values pass through unchanged + valid := []string{"crypto", "shard_01", "a.B-c9", "_", ".-"} + for _, v := range valid { + if got := resolveNamespace(v); got != v { + t.Fatalf("resolveNamespace(%q) = %q, want %q", v, got, v) + } + } + + // empty defaults to "crypto" + if got := resolveNamespace(""); got != "crypto" { + t.Fatalf("resolveNamespace(\"\") = %q, want %q", got, "crypto") + } + + // invalid values are rejected with a clear panic + invalid := []string{"crypto name", "shard/01", "a:b", "ns$", "shard,2", "a=B"} + for _, v := range invalid { + func() { + defer func() { + if rec := recover(); rec == nil { + t.Fatalf("resolveNamespace(%q) did not panic", v) + } + }() + resolveNamespace(v) + }() + } +} From 379f122be4cbe8dd3431ddbfbf9cde89294eb8a8 Mon Sep 17 00:00:00 2001 From: Shane Jaroch Date: Tue, 18 Aug 2026 19:50:09 -0400 Subject: [PATCH 3/6] Update tests/namespace_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- tests/namespace_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/namespace_test.go b/tests/namespace_test.go index 3698161..68199b1 100644 --- a/tests/namespace_test.go +++ b/tests/namespace_test.go @@ -21,9 +21,14 @@ func TestResolveNamespace(t *testing.T) { for _, v := range invalid { func() { defer func() { - if rec := recover(); rec == nil { + rec := recover() + if rec == nil { t.Fatalf("resolveNamespace(%q) did not panic", v) } + want := "COMPLEMENT_CRYPTO_NAMESPACE must contain only characters in [A-Za-z0-9_.-], got: " + v + if rec != want { + t.Fatalf("resolveNamespace(%q) panic = %q, want %q", v, rec, want) + } }() resolveNamespace(v) }() From 5aac0baacb986bab1b65cdd8c59bf495ea312007 Mon Sep 17 00:00:00 2001 From: Shane Jaroch Date: Wed, 19 Aug 2026 16:22:31 -0400 Subject: [PATCH 4/6] fix(justfile): rebuild-rust-sdk injects real crypto feature via cargo.toml patch the _only-for-testing-disable-megolm-minimum-rotation-period-ms feature never existed in matrix-rust-sdk; the _disable-minimum-rotation-period-ms feature lives on matrix-sdk-crypto and cannot be passed through ffi --features. patch the workspace Cargo.toml like upstream rebuild_rust_sdk.sh and restore afterward. --- justfile | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/justfile b/justfile index e7d1de7..f281435 100644 --- a/justfile +++ b/justfile @@ -41,9 +41,18 @@ _build-rust-sdk dir: #!/usr/bin/env bash set -euxo pipefail - cd "{{ dir }}" - - cargo build -p matrix-sdk-ffi --features 'sentry, _only-for-testing-disable-megolm-minimum-rotation-period-ms' + cd "{{ dir }}" + + # The `_disable-minimum-rotation-period-ms` feature lives on matrix-sdk-crypto, + # not on matrix-sdk-ffi, so it cannot be passed via `--features` here. Patch the + # workspace Cargo.toml to inject it into the crypto dep (mirroring upstream + # rebuild_rust_sdk.sh), and restore both files afterwards. + cp Cargo.toml Cargo.toml.backup + cp Cargo.lock Cargo.lock.backup + trap 'mv -f Cargo.toml.backup Cargo.toml; mv -f Cargo.lock.backup Cargo.lock' EXIT + sed -i.bak 's#matrix-sdk-crypto = {#matrix-sdk-crypto = {features = ["_disable-minimum-rotation-period-ms"],#' Cargo.toml + + cargo build -p matrix-sdk-ffi --features 'sentry' uniffi-bindgen-go -o {{ COMPLEMENT_DIR }}/internal/api/rust --config {{ COMPLEMENT_DIR }}/uniffi.toml --library ./target/debug/libmatrix_sdk_ffi.a # Add the cgo LDFLAGS directive to the generated bindings. From 250527b1e5a711bf8d6fe8f37c7af8f0d9664743 Mon Sep 17 00:00:00 2001 From: Shane Jaroch Date: Wed, 19 Aug 2026 20:28:07 -0400 Subject: [PATCH 5/6] test(crypto): retry fallback-key claim to tolerate async SDK upload The SDK uploads its fallback key asynchronously after the sync response tells it one is needed (device_unused_fallback_key_types), so a single immediate /keys/claim can race ahead of the upload and return no key. Retry the claim (matching the WithRetryUntil pattern used elsewhere in this file) instead of failing on the first empty response. Fixes an intermittent TestFallbackKeyIsUsedIfOneTimeKeysRunOut flake in combined runs. --- tests/one_time_keys_test.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/one_time_keys_test.go b/tests/one_time_keys_test.go index 4bcbd60..6d14bef 100644 --- a/tests/one_time_keys_test.go +++ b/tests/one_time_keys_test.go @@ -23,7 +23,12 @@ import ( func mustClaimFallbackKey(t *testing.T, claimer *client.CSAPI, target *cc.User) (fallbackKeyID string, keyJSON gjson.Result) { t.Helper() - res := claimer.MustDo(t, "POST", []string{ + // The SDK uploads its fallback key asynchronously after learning it needs + // one (via device_unused_fallback_key_types in the sync response), so it + // may not have landed yet when the test first claims it. Retry until it + // appears rather than failing on the first (empty) claim. + var result gjson.Result + claimer.MustDo(t, "POST", []string{ "_matrix", "client", "v3", "keys", "claim", }, client.WithJSONBody(t, map[string]any{ "one_time_keys": map[string]any{ @@ -31,9 +36,18 @@ func mustClaimFallbackKey(t *testing.T, claimer *client.CSAPI, target *cc.User) target.DeviceID: "signed_curve25519", }, }, + }), client.WithRetryUntil(10*time.Second, func(res *http.Response) bool { + res.Body.Close() + result = must.ParseJSON(t, res.Body) + otks := result.Get(fmt.Sprintf( + "one_time_keys.%s.%s", client.GjsonEscape(target.UserID), client.GjsonEscape(target.DeviceID), + )) + if otks.Exists() { + return true + } + t.Logf("fallback key not yet uploaded for %s|%s, retrying: %v", target.UserID, target.DeviceID, result.Raw) + return false })) - defer res.Body.Close() - result := must.ParseJSON(t, res.Body) otks := result.Get(fmt.Sprintf( "one_time_keys.%s.%s", client.GjsonEscape(target.UserID), client.GjsonEscape(target.DeviceID), )) From 9277df77b5e7007513c100127e4e90a7aa14f81c Mon Sep 17 00:00:00 2001 From: Shane Jaroch Date: Thu, 20 Aug 2026 00:08:23 -0400 Subject: [PATCH 6/6] Address PR review comments - justfile: Remove stray Cargo.toml.bak and verify sed substitution - tests/one_time_keys_test.go: Fix reading closed response body in mustClaimFallbackKey - tests/one_time_keys_test.go: Add missing docstrings to fix coverage warnings --- justfile | 6 +++++- tests/one_time_keys_test.go | 10 +++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/justfile b/justfile index f281435..46d2210 100644 --- a/justfile +++ b/justfile @@ -51,7 +51,11 @@ _build-rust-sdk dir: cp Cargo.lock Cargo.lock.backup trap 'mv -f Cargo.toml.backup Cargo.toml; mv -f Cargo.lock.backup Cargo.lock' EXIT sed -i.bak 's#matrix-sdk-crypto = {#matrix-sdk-crypto = {features = ["_disable-minimum-rotation-period-ms"],#' Cargo.toml - + rm -f Cargo.toml.bak + if ! grep -q "_disable-minimum-rotation-period-ms" Cargo.toml; then + echo "Failed to inject _disable-minimum-rotation-period-ms feature" >&2 + exit 1 + fi cargo build -p matrix-sdk-ffi --features 'sentry' uniffi-bindgen-go -o {{ COMPLEMENT_DIR }}/internal/api/rust --config {{ COMPLEMENT_DIR }}/uniffi.toml --library ./target/debug/libmatrix_sdk_ffi.a diff --git a/tests/one_time_keys_test.go b/tests/one_time_keys_test.go index 6d14bef..1cf800d 100644 --- a/tests/one_time_keys_test.go +++ b/tests/one_time_keys_test.go @@ -21,6 +21,11 @@ import ( "github.com/tidwall/gjson" ) +// mustClaimFallbackKey claims the fallback key for the target user. +// The SDK uploads its fallback key asynchronously after learning it needs +// one (via device_unused_fallback_key_types in the sync response), so it +// may not have landed yet when the test first claims it. Retry until it +// appears rather than failing on the first (empty) claim. func mustClaimFallbackKey(t *testing.T, claimer *client.CSAPI, target *cc.User) (fallbackKeyID string, keyJSON gjson.Result) { t.Helper() // The SDK uploads its fallback key asynchronously after learning it needs @@ -37,8 +42,8 @@ func mustClaimFallbackKey(t *testing.T, claimer *client.CSAPI, target *cc.User) }, }, }), client.WithRetryUntil(10*time.Second, func(res *http.Response) bool { - res.Body.Close() result = must.ParseJSON(t, res.Body) + res.Body.Close() otks := result.Get(fmt.Sprintf( "one_time_keys.%s.%s", client.GjsonEscape(target.UserID), client.GjsonEscape(target.DeviceID), )) @@ -63,6 +68,7 @@ func mustClaimFallbackKey(t *testing.T, claimer *client.CSAPI, target *cc.User) return fallbackKeyID, fallbackKey } +// mustClaimOTKs repeatedly claims one-time keys for the target user until otkCount keys have been claimed. func mustClaimOTKs(t *testing.T, claimer *client.CSAPI, target *cc.User, otkCount int) { t.Helper() for i := 0; i < otkCount; i++ { @@ -171,6 +177,7 @@ func TestFallbackKeyIsUsedIfOneTimeKeysRunOut(t *testing.T) { }) } +// TestFailedOneTimeKeyUploadRetries tests that the client retries uploading one-time keys if the upload fails. func TestFailedOneTimeKeyUploadRetries(t *testing.T) { Instance().ForEachClientType(t, func(t *testing.T, clientType api.ClientType) { tc := Instance().CreateTestContext(t, clientType, clientType) @@ -219,6 +226,7 @@ func TestFailedOneTimeKeyUploadRetries(t *testing.T) { }) } +// TestFailedKeysClaimRetries tests that the client retries claiming one-time keys if the claim fails. func TestFailedKeysClaimRetries(t *testing.T) { Instance().ForEachClientType(t, func(t *testing.T, clientType api.ClientType) { tc := Instance().CreateTestContext(t, clientType, clientType)