diff --git a/src/Packages/Audience/Runtime/Core/ConsentState.cs b/src/Packages/Audience/Runtime/Core/ConsentState.cs new file mode 100644 index 000000000..d5a7aacce --- /dev/null +++ b/src/Packages/Audience/Runtime/Core/ConsentState.cs @@ -0,0 +1,20 @@ +#nullable enable + +namespace System.Runtime.CompilerServices +{ + // Unity's .NET runtime doesn't include this type, but the C# compiler + // needs it to exist to build the ConsentState record below. Declaring + // an empty one here gives the compiler what it looks for. + internal static class IsExternalInit { } +} + +namespace Immutable.Audience +{ + // Pairs the consent level with the user id so the two always move + // together. Updates swap the whole pair at once — a reader never sees + // the new consent level alongside a leftover user id. + internal sealed record ConsentState(ConsentLevel Level, string? UserId) + { + internal static readonly ConsentState None = new(ConsentLevel.None, null); + } +} diff --git a/src/Packages/Audience/Runtime/ImmutableAudience.cs b/src/Packages/Audience/Runtime/ImmutableAudience.cs index 53027a429..9eaa98503 100644 --- a/src/Packages/Audience/Runtime/ImmutableAudience.cs +++ b/src/Packages/Audience/Runtime/ImmutableAudience.cs @@ -14,9 +14,10 @@ public static class ImmutableAudience { // Reference fields are written inside _initLock; readers check the // `volatile _initialized` flag first so they never see a half-initialised state. - // _consent and _session are written only inside _initLock but read outside, - // so they stay `volatile` to make writes visible across threads. - // _userId is written outside the lock (Identify) — `volatile` for the same reason. + // _state (consent level + userId) and _session are volatile so a write + // on one thread is visible on any other. Every _state write happens + // under _initLock so level and userId always move together — callers + // never observe (Anonymous, oldUserId). // // Init / Shutdown / Reset / SetConsent hold _initLock only to flip state // and capture references; they release the lock before running blocking @@ -30,8 +31,7 @@ public static class ImmutableAudience private static HttpClient? _controlClient; private static CancellationTokenSource? _shutdownCancellationSource; private static Timer? _sendTimer; - private static volatile ConsentLevel _consent; - private static volatile string? _userId; + private static volatile ConsentState _state = ConsentState.None; private static volatile bool _initialized; private static readonly object _initLock = new object(); @@ -76,7 +76,8 @@ public static void Init(AudienceConfig config) _config = config; Log.Enabled = config.Debug; // Persisted consent overrides the config default (prior downgrade survives restart). - _consent = ConsentStore.Load(config.PersistentDataPath) ?? config.Consent; + var initialLevel = ConsentStore.Load(config.PersistentDataPath) ?? config.Consent; + _state = new ConsentState(initialLevel, null); _store = new DiskStore(config.PersistentDataPath); _queue = new EventQueue(_store, config.FlushIntervalSeconds, config.FlushSize); @@ -94,11 +95,11 @@ public static void Init(AudienceConfig config) _initialized = true; // Snapshot so a racing SetConsent(None) can't drop the launch event. - consentAtInit = _consent; + consentAtInit = initialLevel; // Session created under the lock; Start() deferred until after // release because session_start → Track takes its own locks. - if (consentAtInit.CanTrack()) + if (initialLevel.CanTrack()) _session = new Session(Track); // Captured reference: a later SetConsent(None) may dispose this @@ -136,7 +137,8 @@ internal static void OnResume() // IEvent implementations validate required fields at compile time. public static void Track(IEvent evt) { - if (!CanTrack()) return; + var state = _state; + if (!_initialized || !state.Level.CanTrack()) return; if (evt == null) { Log.Warn("Track(IEvent) called with null event — dropping."); @@ -166,10 +168,11 @@ public static void Track(IEvent evt) return; } - var anonymousId = Identity.GetOrCreate(config.PersistentDataPath!, _consent); + var anonymousId = Identity.GetOrCreate(config.PersistentDataPath!, state.Level); // ToProperties returns a fresh dict per call, so no snapshot needed. - var msg = MessageBuilder.Track(eventName, anonymousId, _userId, config.PackageVersion, properties); - Enqueue(msg); + var userId = state.Level == ConsentLevel.Full ? state.UserId : null; + var msg = MessageBuilder.Track(eventName, anonymousId, userId, config.PackageVersion, properties); + EnqueueTrack(msg); } // Sends a custom event. For predefined names (purchase, progression, @@ -177,7 +180,8 @@ public static void Track(IEvent evt) // validates required fields. public static void Track(string eventName, Dictionary? properties = null) { - if (!CanTrack()) return; + var state = _state; + if (!_initialized || !state.Level.CanTrack()) return; if (string.IsNullOrEmpty(eventName)) { Log.Warn("Track(string) called with null or empty event name — dropping."); @@ -187,10 +191,11 @@ public static void Track(string eventName, Dictionary? propertie var config = _config; if (config == null) return; - var anonymousId = Identity.GetOrCreate(config.PersistentDataPath!, _consent); - var msg = MessageBuilder.Track(eventName, anonymousId, _userId, config.PackageVersion, + var anonymousId = Identity.GetOrCreate(config.PersistentDataPath!, state.Level); + var userId = state.Level == ConsentLevel.Full ? state.UserId : null; + var msg = MessageBuilder.Track(eventName, anonymousId, userId, config.PackageVersion, SnapshotCallerDict(properties)); - Enqueue(msg); + EnqueueTrack(msg); } // ----------------------------------------------------------------- @@ -213,21 +218,30 @@ public static void Identify(string userId, string identityType, Dictionary _consent; + internal static ConsentLevel CurrentConsentForTesting => _state.Level; internal static void FlushQueueToDiskForTesting() => _queue?.FlushSync(); @@ -689,23 +711,30 @@ internal static void ResetState() // Private // ----------------------------------------------------------------- - private static bool CanTrack() - { - return _initialized && _consent.CanTrack(); - } - - // Copy the dictionary so the caller editing it later can't corrupt the - // message while the background thread is writing it to disk. + // Shallow-copy the caller's dict so a post-call mutation cannot race the drain-thread serialiser. private static Dictionary? SnapshotCallerDict(Dictionary? src) => src != null ? new Dictionary(src) : null; - private static void Enqueue(Dictionary? msg) + // Checks the current consent inside the drain lock. If consent has + // since dropped to None the message is discarded. If it dropped to + // Anonymous the userId is stripped. + private static void EnqueueTrack(Dictionary? msg) { - var queue = _queue; - if (queue == null) return; + _queue?.EnqueueChecked(msg, m => + { + var state = _state; + if (!state.Level.CanTrack()) return null; + if (state.Level != ConsentLevel.Full) + m.Remove(MessageFields.UserId); + return m; + }); + } - // Re-check consent inside _drainLock so a racing SetConsent(None) can't leak past the purge. - queue.EnqueueChecked(msg, () => _consent.CanTrack()); + // Identify / Alias require Full; drop if consent has downgraded. + private static void EnqueueIdentity(Dictionary? msg) + { + _queue?.EnqueueChecked(msg, m => + _state.Level == ConsentLevel.Full ? m : null); } private static void SendBatch() @@ -761,7 +790,7 @@ private static void RescheduleSendTimer(HttpTransport transport) timer.Change(nextMs, sendIntervalMs); } - // consentAtInit only gates the launch; Track still checks live _consent via CanTrack. + // consentAtInit only gates the launch; Track still checks live _state via CanTrack. private static void FireGameLaunch(AudienceConfig config, ConsentLevel consentAtInit) { if (!consentAtInit.CanTrack()) return; diff --git a/src/Packages/Audience/Runtime/Transport/EventQueue.cs b/src/Packages/Audience/Runtime/Transport/EventQueue.cs index e81fcde43..66922e3f0 100644 --- a/src/Packages/Audience/Runtime/Transport/EventQueue.cs +++ b/src/Packages/Audience/Runtime/Transport/EventQueue.cs @@ -64,16 +64,26 @@ internal void Enqueue(Dictionary? msg) _flushGate.Set(); } - // Enqueues under _drainLock, re-checking stillAllowed inside the lock. - // Closes the window where a concurrent PurgeAll could complete between - // the caller's check and the enqueue, leaking the event past revocation. - internal void EnqueueChecked(Dictionary? msg, Func? stillAllowed) + // Queues the message under the drain lock. The caller supplies a + // transform that runs while the lock is held — it can edit the + // message or return null to drop it. Running under the lock means + // PurgeAll and ApplyAnonymousDowngrade can't slip in mid-decision, so + // a Track that races a consent downgrade gets its userId stripped or + // the message dropped before it reaches the queue. + internal void EnqueueChecked( + Dictionary? msg, + Func, Dictionary?>? transform) { if (_disposed || msg == null) return; lock (_drainLock) { - if (stillAllowed != null && !stillAllowed()) return; + if (transform != null) + { + var transformed = transform(msg); + if (transformed == null) return; + msg = transformed; + } _memory.Enqueue(msg); } diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index 8cd9f491d..e71f19c76 100644 --- a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs +++ b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs @@ -568,7 +568,7 @@ public void SetConsent_DowngradeToNone_StressTest_NoLeak() // before SetConsent starts on a fast machine. This stress variant runs // the race many times with many concurrent Track threads so at least // some iterations are guaranteed to land the enqueue inside the - // _consent=None/PurgeAll window. + // _state=None/PurgeAll window. // // Without the EnqueueChecked re-check, this test leaks events // reproducibly. With the fix, zero leaks across all iterations. @@ -748,6 +748,72 @@ public void SetConsent_ConcurrentUpgradeFromNone_StartsOneSession_StressTest() } } + [Test] + public void SetConsent_DowngradeToAnonymous_StressTest_NoUserIdLeak() + { + // Full → Anonymous race: Track reads _state with userId still set, + // then SetConsent flips _state to Anonymous and calls + // ApplyAnonymousDowngrade (one-shot rewrite). If Track's enqueue + // lands after the rewrite, the msg with userId is not stripped. + // + // With the ConsentState + EnqueueChecked transform in place, Track's + // transform runs under _drainLock and strips userId when current state + // is not Full. Zero leaks across all iterations. + // + // Sabotage: remove the `m.Remove(MessageFields.UserId)` in + // EnqueueTrack and this test leaks reproducibly. + const int iterations = 200; + const int trackersPerIteration = 4; + const string testUserId = "user_race_stress"; + + for (int iter = 0; iter < iterations; iter++) + { + ImmutableAudience.Init(MakeConfig(ConsentLevel.Full)); + ImmutableAudience.Identify(testUserId, "steam"); + + // Clear Init events so only race events can leak. + ImmutableAudience.FlushQueueToDiskForTesting(); + var queueDir = AudiencePaths.QueueDir(_testDir); + if (Directory.Exists(queueDir)) + foreach (var f in Directory.GetFiles(queueDir, "*.json")) File.Delete(f); + + var barrier = new Barrier(trackersPerIteration + 1); + var trackers = new Task[trackersPerIteration]; + for (int t = 0; t < trackersPerIteration; t++) + { + trackers[t] = Task.Run(() => + { + barrier.SignalAndWait(); + ImmutableAudience.Track("race_stress"); + }); + } + + barrier.SignalAndWait(); + ImmutableAudience.SetConsent(ConsentLevel.Anonymous); + Task.WaitAll(trackers, TimeSpan.FromSeconds(5)); + + ImmutableAudience.FlushQueueToDiskForTesting(); + + int userIdLeaks = 0; + if (Directory.Exists(queueDir)) + { + userIdLeaks = Directory.GetFiles(queueDir, "*.json") + .Select(File.ReadAllText) + .Count(c => c.Contains($"\"{testUserId}\"")); + } + + if (userIdLeaks > 0) + { + Assert.Fail( + $"iteration {iter}: {userIdLeaks} track events retained userId past SetConsent(Anonymous)"); + } + + ImmutableAudience.ResetState(); + if (Directory.Exists(AudiencePaths.AudienceDir(_testDir))) + Directory.Delete(AudiencePaths.AudienceDir(_testDir), recursive: true); + } + } + [Test] public void ResetState_ClearsIdentityCache_AcrossInitWithDifferentPath() {