Skip to content

identity: areMe converts storage errors into false-negative ownership answers - #2172

Draft
Effi-S wants to merge 2 commits into
mainfrom
fix-2066
Draft

identity: areMe converts storage errors into false-negative ownership answers#2172
Effi-S wants to merge 2 commits into
mainfrom
fix-2066

Conversation

@Effi-S

@Effi-S Effi-S commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #2066

Summary

Provider.areMe (backing both AreMe and IsMe) returns whatever partial result it has accumulated
when the storage lookup for signer existence errors, rather than propagating the error. This means a
transient storage failure makes IsMe report false ("not mine") for an identity that actually is
ours, which is on the token-ownership decision path.

Where

token/services/identity/provider.go:270-279:

// check Storage
found, err := p.storage.GetExistingSignerInfo(ctx, notFound...)
if err != nil {
	p.Logger.Errorf("failed checking if a signer exists [%s]", err)
	return result.ToSlice()
}
result.Add(found...)
return result.ToSlice()

On error, result contains only whatever was already resolved from the in-memory cache
(provider.go:257-264) before the storage call — everything not in cache is silently reported as
"not mine," logged only at Errorf with no error propagated to the caller.

Related: token/services/identity/wallet/service.go:138-149 Wallet() similarly discards two errors
from OwnerWallet/IssuerWallet and returns nil (a caller cannot distinguish "no wallet" from
"lookup failed"):

func (s *Service) Wallet(ctx context.Context, identity tdriver.Identity) tdriver.Wallet {
	w, _ := s.OwnerWallet(ctx, identity)
	if w != nil {
		return w
	}
	iw, _ := s.IssuerWallet(ctx, identity)
	if iw != nil {
		return iw
	}
	return nil
}

Impact

IsMe is used to decide ownership-related behavior (e.g. whether to react to a token as an owned
token). A transient storage error causing a false negative means an owned token can be treated as
not-owned for that call, with no visible error — the caller has no signal that the answer is
unreliable rather than authoritative.

Reproduction

Not yet committed. Mock Storage.GetExistingSignerInfo to return an error for a set of identities
that includes at least one genuinely-owned identity not already warm in the in-memory cache; assert
IsMe/AreMe either propagates the error or is documented as best-effort at the call sites that rely
on it for correctness-sensitive decisions.

Suggested fix

AreMe/IsMe's current signatures ([]string / bool, no error return) make it structurally unable
to distinguish "confirmed not mine" from "couldn't check." Changing the signature ripples through the
driver.IdentityProvider interface and every caller, so the minimal fix here is: keep the signatures,
but do not silently swallow storage errors into a negative result for the specific identities the
storage call was about
— instead of returning early, consider retrying once, or making the
error-swallowing explicit and loud enough (metric increment, not just a log line) that an operator can
detect ownership answers were degraded. If callers need a hard guarantee, a follow-up to thread an
error return through AreMe is worth considering separately.

Severity

MEDIUM — requires a transient storage failure to trigger, and produces an unsignaled false negative
on an ownership decision.

Effi-S added 2 commits August 10, 2026 10:18
Signed-off-by: Effi-S <effi.szt@gmail.com>
Signed-off-by: Effi-S <effi.szt@gmail.com>
@Effi-S Effi-S added this to the Q3/26 milestone Aug 10, 2026
@Effi-S Effi-S self-assigned this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

identity: areMe converts storage errors into false-negative ownership answers

1 participant