From e83c8ab54544a3fad6f3783e52ee56906d6fc972 Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Mon, 5 Jun 2023 22:09:02 -0700 Subject: [PATCH 1/9] Update EventPropertiesDecorator.hpp Scrub IP addresses by default. --- lib/decorators/EventPropertiesDecorator.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/decorators/EventPropertiesDecorator.hpp b/lib/decorators/EventPropertiesDecorator.hpp index 95ef92304..7de12eedb 100644 --- a/lib/decorators/EventPropertiesDecorator.hpp +++ b/lib/decorators/EventPropertiesDecorator.hpp @@ -24,6 +24,7 @@ namespace MAT_NS_BEGIN { #define RECORD_FLAGS_EVENTTAG_HASH_PII 0x00100000 // #define MICROSOFT_EVENTTAG_DROP_PII 0x02000000 #define RECORD_FLAGS_EVENTTAG_DROP_PII 0x00200000 +#define RECORD_FLAGS_EVENTTAG_SCRUB_IP 0x00400000 class EventPropertiesDecorator : public IDecorator { @@ -124,6 +125,8 @@ namespace MAT_NS_BEGIN { int64_t tags = eventProperties.GetPolicyBitFlags(); int64_t flags = 0; + // Scrub/obfuscate IP addresses. + flags = RECORD_FLAGS_EVENTTAG_SCRUB_IP; // We must remap from one bitfield set to another, no way to bit-shift :( // At the moment 1DS SDK in direct upload mode supports DROP and MARK tags only: flags |= (tags & MICROSOFT_EVENTTAG_MARK_PII) ? RECORD_FLAGS_EVENTTAG_MARK_PII : 0; From 1ec904da9901b7a959115e98c4627e0f90cd84f4 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 10 Jun 2026 18:14:15 -0500 Subject: [PATCH 2/9] Gate IP scrubbing behind CFG_BOOL_ENABLE_IP_SCRUBBING (default on) The initial change set RECORD_FLAGS_EVENTTAG_SCRUB_IP unconditionally for every event, forcing IP scrubbing on all SDK consumers in direct-upload mode -- a breaking change for apps that need client IP (e.g. geo-location). Make scrubbing the default but opt-out: the decorator sets the SCRUB_IP record flag unless CFG_BOOL_ENABLE_IP_SCRUBBING is explicitly set to false. record.flags is forwarded on the cross-platform/direct-upload path, so this redacts client IP at the collector without relying on ext.metadata privacy tags. - ILogConfiguration.hpp: add CFG_BOOL_ENABLE_IP_SCRUBBING config key - EventPropertiesDecorator.hpp: gate SCRUB_IP flag behind the config - EventPropertiesDecoratorTests.cpp: add default-on, opt-out, explicit-enable tests with a per-instance ConfigurableLogManager helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/decorators/EventPropertiesDecorator.hpp | 10 +++- lib/include/public/ILogConfiguration.hpp | 7 +++ .../EventPropertiesDecoratorTests.cpp | 51 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/lib/decorators/EventPropertiesDecorator.hpp b/lib/decorators/EventPropertiesDecorator.hpp index 9ff6da16c..d454849dc 100644 --- a/lib/decorators/EventPropertiesDecorator.hpp +++ b/lib/decorators/EventPropertiesDecorator.hpp @@ -126,8 +126,14 @@ namespace MAT_NS_BEGIN { int64_t tags = eventProperties.GetPolicyBitFlags(); int64_t flags = 0; - // Scrub/obfuscate IP addresses. - flags = RECORD_FLAGS_EVENTTAG_SCRUB_IP; + // Scrub/obfuscate the client IP address at the collector by default. + // Hosts that require the client IP (e.g. for geo-location enrichment) + // can opt out by setting CFG_BOOL_ENABLE_IP_SCRUBBING = false. + ILogConfiguration& config = m_owner.GetLogConfiguration(); + if (!config.HasConfig(CFG_BOOL_ENABLE_IP_SCRUBBING) || config[CFG_BOOL_ENABLE_IP_SCRUBBING]) + { + flags |= RECORD_FLAGS_EVENTTAG_SCRUB_IP; + } // We must remap from one bitfield set to another, no way to bit-shift :( // At the moment 1DS SDK in direct upload mode supports DROP and MARK tags only: flags |= (tags & MICROSOFT_EVENTTAG_MARK_PII) ? RECORD_FLAGS_EVENTTAG_MARK_PII : 0; diff --git a/lib/include/public/ILogConfiguration.hpp b/lib/include/public/ILogConfiguration.hpp index 1cb8103b8..555f33f5c 100644 --- a/lib/include/public/ILogConfiguration.hpp +++ b/lib/include/public/ILogConfiguration.hpp @@ -104,6 +104,13 @@ namespace MAT_NS_BEGIN /// static constexpr const char* const CFG_BOOL_ENABLE_NET_DETECT = "enableNetworkDetector"; + /// + /// Scrub (obfuscate) the client IP address at the collector for telemetry + /// uploaded in direct-upload mode. Enabled by default; set to false to opt + /// out (e.g. when the client IP is required for geo-location enrichment). + /// + static constexpr const char* const CFG_BOOL_ENABLE_IP_SCRUBBING = "enableIpScrubbing"; + /// /// Parameter that allows to check if the SDK is running on UTC mode /// diff --git a/tests/unittests/EventPropertiesDecoratorTests.cpp b/tests/unittests/EventPropertiesDecoratorTests.cpp index f38444fd0..348354604 100644 --- a/tests/unittests/EventPropertiesDecoratorTests.cpp +++ b/tests/unittests/EventPropertiesDecoratorTests.cpp @@ -28,6 +28,19 @@ class TestEventPropertiesDecorator : public EventPropertiesDecorator } }; +// NullLogManager hands out a single shared static ILogConfiguration, which would +// leak configuration across tests. This subclass owns a per-instance configuration +// so the IP-scrubbing opt-out can be exercised in isolation. +class ConfigurableLogManager : public NullLogManager +{ +public: + ILogConfiguration localConfig; + ILogConfiguration& GetLogConfiguration() override + { + return localConfig; + } +}; + static std::unique_ptr PopulateRecordForDropPii() { auto record = std::unique_ptr(new Record{}); @@ -545,3 +558,41 @@ TEST(EventPropertiesDecoratorTests, DropPiiPartA_StripsValues) EXPECT_THAT(record->extSdk[0].installId, Eq("")); EXPECT_THAT(record->cV, Eq("")); } + +TEST(EventPropertiesDecoratorTests, Decorate_ScrubIp_EnabledByDefault) +{ + ConfigurableLogManager logManager; // CFG_BOOL_ENABLE_IP_SCRUBBING not set + EventPropertiesDecorator decorator(logManager); + Record record; + EventProperties props {"TestEvent"}; + EventLatency latency = EventLatency::EventLatency_Normal; + + EXPECT_TRUE(decorator.decorate(record, latency, props)); + EXPECT_TRUE(record.flags & RECORD_FLAGS_EVENTTAG_SCRUB_IP); +} + +TEST(EventPropertiesDecoratorTests, Decorate_ScrubIp_OptOutViaConfig) +{ + ConfigurableLogManager logManager; + logManager.localConfig[CFG_BOOL_ENABLE_IP_SCRUBBING] = false; + EventPropertiesDecorator decorator(logManager); + Record record; + EventProperties props {"TestEvent"}; + EventLatency latency = EventLatency::EventLatency_Normal; + + EXPECT_TRUE(decorator.decorate(record, latency, props)); + EXPECT_FALSE(record.flags & RECORD_FLAGS_EVENTTAG_SCRUB_IP); +} + +TEST(EventPropertiesDecoratorTests, Decorate_ScrubIp_ExplicitlyEnabled) +{ + ConfigurableLogManager logManager; + logManager.localConfig[CFG_BOOL_ENABLE_IP_SCRUBBING] = true; + EventPropertiesDecorator decorator(logManager); + Record record; + EventProperties props {"TestEvent"}; + EventLatency latency = EventLatency::EventLatency_Normal; + + EXPECT_TRUE(decorator.decorate(record, latency, props)); + EXPECT_TRUE(record.flags & RECORD_FLAGS_EVENTTAG_SCRUB_IP); +} From 0f4ad995c8f2a74ed235a11df6a61829b525da28 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 10 Jun 2026 19:33:51 -0500 Subject: [PATCH 3/9] Cover SDK stats events and add wrapper-parity config keys for IP scrubbing - Statistics.cpp: SDK statistics/metastats events bypass EventPropertiesDecorator, so apply the same collector-side client-IP scrub (gated by CFG_BOOL_ENABLE_IP_SCRUBBING, on by default) to those records too. Closes the gap identified in PR review. - LogConfigurationKey.java + ODWLogConfiguration.{h,mm}: expose CFG_BOOL_ENABLE_IP_SCRUBBING ('enableIpScrubbing') to the Android (Java) and Apple (Obj-C) wrappers for API parity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../applications/events/LogConfigurationKey.java | 3 +++ lib/stats/Statistics.cpp | 8 ++++++++ wrappers/obj-c/ODWLogConfiguration.h | 5 +++++ wrappers/obj-c/ODWLogConfiguration.mm | 5 +++++ 4 files changed, 21 insertions(+) diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java index 329f17680..868966186 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java @@ -29,6 +29,9 @@ public enum LogConfigurationKey { /** Enable network detector. */ CFG_BOOL_ENABLE_NET_DETECT("enableNetworkDetector", Boolean.class), + /** Scrub (obfuscate) the client IP address at the collector. Enabled by default; set to false to opt out. */ + CFG_BOOL_ENABLE_IP_SCRUBBING("enableIpScrubbing", Boolean.class), + CFG_BOOL_TPM_CLOCK_SKEW_ENABLED("clockSkewEnabled", Boolean.class), /** Parameter that allows to check if the SDK is running on UTC mode */ diff --git a/lib/stats/Statistics.cpp b/lib/stats/Statistics.cpp index 773dd4d13..4c699f759 100644 --- a/lib/stats/Statistics.cpp +++ b/lib/stats/Statistics.cpp @@ -9,6 +9,7 @@ #include "ILogManager.hpp" #include "mat/config.h" #include "utils/Utils.hpp" +#include "decorators/EventPropertiesDecorator.hpp" #include namespace MAT_NS_BEGIN { @@ -83,6 +84,13 @@ namespace MAT_NS_BEGIN { result &= m_baseDecorator.decorate(record); // Allow stats to capture Part A common properties, but not the custom result &= m_semanticContextDecorator.decorate(record, true); + // Stats events bypass EventPropertiesDecorator, so apply the same + // collector-side client-IP scrub here (on by default; opt out via + // CFG_BOOL_ENABLE_IP_SCRUBBING = false). + if (!m_config.HasConfig(CFG_BOOL_ENABLE_IP_SCRUBBING) || m_config[CFG_BOOL_ENABLE_IP_SCRUBBING]) + { + record.flags |= RECORD_FLAGS_EVENTTAG_SCRUB_IP; + } if (result) { IncomingEventContext evt(PAL::generateUuidString(), tenantToken, EventLatency_Normal, EventPersistence_Normal, &record); diff --git a/wrappers/obj-c/ODWLogConfiguration.h b/wrappers/obj-c/ODWLogConfiguration.h index 3cbdaaa61..dd7b944b0 100644 --- a/wrappers/obj-c/ODWLogConfiguration.h +++ b/wrappers/obj-c/ODWLogConfiguration.h @@ -44,6 +44,11 @@ extern NSString * _Nonnull const ODWCFG_BOOL_ENABLE_WAL_JOURNAL; */ extern NSString * _Nonnull const ODWCFG_BOOL_ENABLE_NET_DETECT; +/*! + Scrub (obfuscate) the client IP address at the collector. Enabled by default; set to false to opt out. +*/ +extern NSString * _Nonnull const ODWCFG_BOOL_ENABLE_IP_SCRUBBING; + /*! The event collection URI. */ diff --git a/wrappers/obj-c/ODWLogConfiguration.mm b/wrappers/obj-c/ODWLogConfiguration.mm index d69ddf70c..507f828a1 100644 --- a/wrappers/obj-c/ODWLogConfiguration.mm +++ b/wrappers/obj-c/ODWLogConfiguration.mm @@ -50,6 +50,11 @@ */ NSString *const ODWCFG_BOOL_ENABLE_NET_DETECT = @"enableNetworkDetector"; +/*! + Scrub (obfuscate) the client IP address at the collector. Enabled by default; set to false to opt out. +*/ +NSString *const ODWCFG_BOOL_ENABLE_IP_SCRUBBING = @"enableIpScrubbing"; + /*! The event collection URI. */ From a0fd8c934f1339c2ccb821a5c273e24bb32a0017 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 10 Jun 2026 20:43:41 -0500 Subject: [PATCH 4/9] Clarify CFG_BOOL_ENABLE_IP_SCRUBBING docstring (Copilot round 1) The doc implied the setting only applies in direct-upload mode, but the scrub flag is set for all events and modes. Clarify that the flag is honored by the OneCollector direct-upload path while UTC mode applies its own client-privacy handling. No behavior change -- the flag is intentionally mode-agnostic and is ignored by the UTC pipeline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/include/public/ILogConfiguration.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/include/public/ILogConfiguration.hpp b/lib/include/public/ILogConfiguration.hpp index 555f33f5c..c36f685d3 100644 --- a/lib/include/public/ILogConfiguration.hpp +++ b/lib/include/public/ILogConfiguration.hpp @@ -105,9 +105,11 @@ namespace MAT_NS_BEGIN static constexpr const char* const CFG_BOOL_ENABLE_NET_DETECT = "enableNetworkDetector"; /// - /// Scrub (obfuscate) the client IP address at the collector for telemetry - /// uploaded in direct-upload mode. Enabled by default; set to false to opt - /// out (e.g. when the client IP is required for geo-location enrichment). + /// Request collector-side scrubbing (obfuscation) of the client IP address. + /// Enabled by default; set to false to opt out (e.g. when the client IP is + /// needed for geo-location enrichment). The scrub flag is honored by the + /// OneCollector direct-upload path; in UTC mode client privacy is governed + /// by the OS UTC pipeline instead. /// static constexpr const char* const CFG_BOOL_ENABLE_IP_SCRUBBING = "enableIpScrubbing"; From 1a6c5c3a390f53131285d74cb03a83e69320b515 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 10 Jun 2026 20:55:12 -0500 Subject: [PATCH 5/9] Address Copilot round 2 on #1161: self-contained header + default-config docs - EventPropertiesDecorator.hpp: include ILogManager.hpp so the header is self-contained for the ILogManager (m_owner) and ILogConfiguration types / CFG_BOOL_ENABLE_IP_SCRUBBING used inline, instead of relying on the includer. - Clarify CFG_BOOL_ENABLE_IP_SCRUBBING docs (C++ / Java / Obj-C): scrubbing is applied unless explicitly set to false (on by default) and the key is not present in the default configuration, so GetDefaultConfiguration() does not surface it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../applications/events/LogConfigurationKey.java | 2 +- lib/decorators/EventPropertiesDecorator.hpp | 1 + lib/include/public/ILogConfiguration.hpp | 9 +++++---- wrappers/obj-c/ODWLogConfiguration.h | 2 +- wrappers/obj-c/ODWLogConfiguration.mm | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java index 868966186..0ce2881ef 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java @@ -29,7 +29,7 @@ public enum LogConfigurationKey { /** Enable network detector. */ CFG_BOOL_ENABLE_NET_DETECT("enableNetworkDetector", Boolean.class), - /** Scrub (obfuscate) the client IP address at the collector. Enabled by default; set to false to opt out. */ + /** Scrub (obfuscate) the client IP address at the collector. Applied unless explicitly set to false (on by default; not present in the default configuration). */ CFG_BOOL_ENABLE_IP_SCRUBBING("enableIpScrubbing", Boolean.class), CFG_BOOL_TPM_CLOCK_SKEW_ENABLED("clockSkewEnabled", Boolean.class), diff --git a/lib/decorators/EventPropertiesDecorator.hpp b/lib/decorators/EventPropertiesDecorator.hpp index d454849dc..663045e04 100644 --- a/lib/decorators/EventPropertiesDecorator.hpp +++ b/lib/decorators/EventPropertiesDecorator.hpp @@ -6,6 +6,7 @@ #define EVENTPROPERTIESDECORATOR_HPP #include "IDecorator.hpp" +#include "ILogManager.hpp" #include "EventProperties.hpp" #include "CorrelationVector.hpp" #include "utils/Utils.hpp" diff --git a/lib/include/public/ILogConfiguration.hpp b/lib/include/public/ILogConfiguration.hpp index c36f685d3..af1bc44c2 100644 --- a/lib/include/public/ILogConfiguration.hpp +++ b/lib/include/public/ILogConfiguration.hpp @@ -106,10 +106,11 @@ namespace MAT_NS_BEGIN /// /// Request collector-side scrubbing (obfuscation) of the client IP address. - /// Enabled by default; set to false to opt out (e.g. when the client IP is - /// needed for geo-location enrichment). The scrub flag is honored by the - /// OneCollector direct-upload path; in UTC mode client privacy is governed - /// by the OS UTC pipeline instead. + /// Applied unless explicitly set to false (on by default; the key is not + /// present in the default configuration). Opt out when the client IP is + /// needed, e.g. for geo-location enrichment. Honored by the OneCollector + /// direct-upload path; in UTC mode client privacy is governed by the OS UTC + /// pipeline instead. /// static constexpr const char* const CFG_BOOL_ENABLE_IP_SCRUBBING = "enableIpScrubbing"; diff --git a/wrappers/obj-c/ODWLogConfiguration.h b/wrappers/obj-c/ODWLogConfiguration.h index dd7b944b0..6e3f77946 100644 --- a/wrappers/obj-c/ODWLogConfiguration.h +++ b/wrappers/obj-c/ODWLogConfiguration.h @@ -45,7 +45,7 @@ extern NSString * _Nonnull const ODWCFG_BOOL_ENABLE_WAL_JOURNAL; extern NSString * _Nonnull const ODWCFG_BOOL_ENABLE_NET_DETECT; /*! - Scrub (obfuscate) the client IP address at the collector. Enabled by default; set to false to opt out. + Scrub (obfuscate) the client IP address at the collector. Applied unless explicitly set to false (on by default; not present in the default configuration). */ extern NSString * _Nonnull const ODWCFG_BOOL_ENABLE_IP_SCRUBBING; diff --git a/wrappers/obj-c/ODWLogConfiguration.mm b/wrappers/obj-c/ODWLogConfiguration.mm index 507f828a1..611b92940 100644 --- a/wrappers/obj-c/ODWLogConfiguration.mm +++ b/wrappers/obj-c/ODWLogConfiguration.mm @@ -51,7 +51,7 @@ NSString *const ODWCFG_BOOL_ENABLE_NET_DETECT = @"enableNetworkDetector"; /*! - Scrub (obfuscate) the client IP address at the collector. Enabled by default; set to false to opt out. + Scrub (obfuscate) the client IP address at the collector. Applied unless explicitly set to false (on by default; not present in the default configuration). */ NSString *const ODWCFG_BOOL_ENABLE_IP_SCRUBBING = @"enableIpScrubbing"; From 64e6181a96cc8d526d9a00266729d889cb5c4504 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Wed, 10 Jun 2026 21:06:32 -0500 Subject: [PATCH 6/9] Address Copilot round 3 on #1161: extract on-wire record flags to a shared header Move the RECORD_FLAGS_EVENTTAG_* on-wire bits out of EventPropertiesDecorator.hpp into a dedicated decorators/RecordFlagConstants.hpp, exposed as static constexpr std::int64_t in the MAT namespace (no longer #define macros). This lets the stats pipeline reference RECORD_FLAGS_EVENTTAG_SCRUB_IP via the small shared header instead of pulling in the full decorator header, and avoids macro pollution. - New: lib/decorators/RecordFlagConstants.hpp - EventPropertiesDecorator.hpp: include the shared header; drop the macros - Statistics.cpp: include the shared header instead of EventPropertiesDecorator.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/decorators/EventPropertiesDecorator.hpp | 11 +------- lib/decorators/RecordFlagConstants.hpp | 30 +++++++++++++++++++++ lib/stats/Statistics.cpp | 2 +- 3 files changed, 32 insertions(+), 11 deletions(-) create mode 100644 lib/decorators/RecordFlagConstants.hpp diff --git a/lib/decorators/EventPropertiesDecorator.hpp b/lib/decorators/EventPropertiesDecorator.hpp index 663045e04..800dc1635 100644 --- a/lib/decorators/EventPropertiesDecorator.hpp +++ b/lib/decorators/EventPropertiesDecorator.hpp @@ -7,6 +7,7 @@ #include "IDecorator.hpp" #include "ILogManager.hpp" +#include "RecordFlagConstants.hpp" #include "EventProperties.hpp" #include "CorrelationVector.hpp" #include "utils/Utils.hpp" @@ -17,16 +18,6 @@ namespace MAT_NS_BEGIN { -// Bit remapping has to happen on bits passed via API surface. -// Ref CS2.1+ : https://osgwiki.com/wiki/CommonSchema/flags -// #define MICROSOFT_EVENTTAG_MARK_PII 0x08000000 -#define RECORD_FLAGS_EVENTTAG_MARK_PII 0x00080000 -// #define MICROSOFT_EVENTTAG_HASH_PII 0x04000000 -#define RECORD_FLAGS_EVENTTAG_HASH_PII 0x00100000 -// #define MICROSOFT_EVENTTAG_DROP_PII 0x02000000 -#define RECORD_FLAGS_EVENTTAG_DROP_PII 0x00200000 -#define RECORD_FLAGS_EVENTTAG_SCRUB_IP 0x00400000 - class EventPropertiesDecorator : public IDecorator { protected: diff --git a/lib/decorators/RecordFlagConstants.hpp b/lib/decorators/RecordFlagConstants.hpp new file mode 100644 index 000000000..8c0fe56d3 --- /dev/null +++ b/lib/decorators/RecordFlagConstants.hpp @@ -0,0 +1,30 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#ifndef RECORDFLAGCONSTANTS_HPP +#define RECORDFLAGCONSTANTS_HPP + +#include "ctmacros.hpp" + +#include + +namespace MAT_NS_BEGIN { + + // On-wire CS protocol record.flags bits. These are distinct from the + // API-surface MICROSOFT_EVENTTAG_* policy flags and are remapped onto + // record.flags by EventPropertiesDecorator. Kept in a dedicated header so + // other components (e.g. the stats pipeline) can reference a single bit + // definition without depending on the decorator's inline implementation. + // Ref CS2.1+: https://osgwiki.com/wiki/CommonSchema/flags + // (API-surface MICROSOFT_EVENTTAG_MARK_PII 0x08000000) + static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_MARK_PII = 0x00080000; + // (API-surface MICROSOFT_EVENTTAG_HASH_PII 0x04000000) + static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_HASH_PII = 0x00100000; + // (API-surface MICROSOFT_EVENTTAG_DROP_PII 0x02000000) + static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_DROP_PII = 0x00200000; + static constexpr std::int64_t RECORD_FLAGS_EVENTTAG_SCRUB_IP = 0x00400000; + +} MAT_NS_END + +#endif // RECORDFLAGCONSTANTS_HPP diff --git a/lib/stats/Statistics.cpp b/lib/stats/Statistics.cpp index 4c699f759..a1377ac37 100644 --- a/lib/stats/Statistics.cpp +++ b/lib/stats/Statistics.cpp @@ -9,7 +9,7 @@ #include "ILogManager.hpp" #include "mat/config.h" #include "utils/Utils.hpp" -#include "decorators/EventPropertiesDecorator.hpp" +#include "decorators/RecordFlagConstants.hpp" #include namespace MAT_NS_BEGIN { From afc0a56a59f1644a76bb5ceb906714b347943001 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 16 Jun 2026 03:20:30 -0500 Subject: [PATCH 7/9] Harden OfflineStorage_Room against null JNI objects (no-crash on null m_room/element) Defensive follow-up to the #1227 family of Android Room crashes ("java_object == null in call to GetObjectClass" under GetAndReserveRecords). #1417 fixed the stale-local-ref root cause but left two null paths unguarded, which can still hard-abort the host process: * m_room is null when the Room DB failed to open or was torn down (the destructor already guards with `if (s_vm && m_room)`, but ~10 other methods dereferenced it unconditionally). Add `if (!m_room) return ;` guards to DeleteRecords(x2), GetAndReserveRecords, ReleaseRecords, StoreRecords, DeleteSetting, StoreSetting, GetSizeInternal, GetRecordCount, ResizeDbInternal, and GetRecords. GetSetting already had this guard. * a null element in the getAndReserve/releaseRecords result array (observed to be androidx.room-version sensitive) was passed to GetObjectClass. Guard both loops: GetAndReserveRecords pops the frame and stops (the existing index < limit path releases the rest for retry); ReleaseRecords skips it. Turns a process-killing JNI abort into graceful degradation. No functional change on the healthy path. Compiles on Android only (JNI/Room) -- validated by CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/offline/OfflineStorage_Room.cpp | 59 +++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/lib/offline/OfflineStorage_Room.cpp b/lib/offline/OfflineStorage_Room.cpp index 72a04d0ed..a2decde1f 100644 --- a/lib/offline/OfflineStorage_Room.cpp +++ b/lib/offline/OfflineStorage_Room.cpp @@ -240,6 +240,10 @@ namespace MAT_NS_BEGIN MATSDK_THROW(std::logic_error("whereFilter not implemented")); } + if (!m_room) + { + return; + } auto room_class = env->GetObjectClass(m_room); auto deleteByToken = env->GetMethodID(room_class, "deleteByToken", @@ -274,6 +278,10 @@ namespace MAT_NS_BEGIN { return; } + if (!m_room) + { + return; + } auto room_class = env->GetObjectClass(m_room); auto method = env->GetMethodID(room_class, "deleteById", "([J)J"); ThrowLogic(env, "Unable to get deleteById method"); @@ -377,6 +385,10 @@ namespace MAT_NS_BEGIN { return false; } + if (!m_room) + { + return false; + } auto room_class = env->GetObjectClass(m_room); auto reserve = env->GetMethodID(room_class, "getAndReserve", "(IJJJ)[Lcom/microsoft/applications/events/StorageRecord;"); @@ -429,6 +441,15 @@ namespace MAT_NS_BEGIN env.pushLocalFrame(32); auto record = env->GetObjectArrayElement(selected, index); ThrowLogic(env, "getAndReserve element"); + if (!record) + { + // Null array element (observed with some androidx.room + // versions): pop this frame and stop. index < limit below + // releases the rest for retry rather than dereferencing + // null in GetObjectClass. + env.popLocalFrame(); + break; + } if (!record_class) { // Promote to a global ref so it survives popLocalFrame on @@ -633,6 +654,10 @@ namespace MAT_NS_BEGIN try { ConnectedEnv env(s_vm); + if (!m_room) + { + return; + } auto room_class = env->GetObjectClass(m_room); ThrowLogic(env, "GetObjectClass for m_room"); auto release = env->GetMethodID(room_class, @@ -700,6 +725,12 @@ namespace MAT_NS_BEGIN env.pushLocalFrame(8); auto byTenant = env->GetObjectArrayElement(results, index); ThrowRuntime(env, "Exception fetching element from results"); + if (!byTenant) + { + // Skip a null array element rather than dereference null. + env.popLocalFrame(); + continue; + } if (!bt_class) { // Promote to a global ref so it survives popLocalFrame. @@ -794,6 +825,10 @@ namespace MAT_NS_BEGIN static constexpr char newRecordSignature[] = "(JIIJIJ[B)Lcom/microsoft/applications/events/StorageRecord;"; + if (!m_room) + { + return 0; + } auto room_class = env->GetObjectClass(m_room); size_t count = std::min(records.size(), INT32_MAX); @@ -924,6 +959,10 @@ namespace MAT_NS_BEGIN try { ConnectedEnv env(s_vm); + if (!m_room) + { + return false; + } auto room_class = env->GetObjectClass(m_room); auto delete_method = env->GetMethodID(room_class, "deleteSetting", "(Ljava/lang/String;)V"); @@ -964,6 +1003,10 @@ namespace MAT_NS_BEGIN try { ConnectedEnv env(s_vm); + if (!m_room) + { + return false; + } auto room_class = env->GetObjectClass(m_room); jmethodID store_setting = env->GetMethodID( room_class, @@ -1081,6 +1124,10 @@ namespace MAT_NS_BEGIN size_t OfflineStorage_Room::GetSizeInternal(ConnectedEnv& env) const { + if (!m_room) + { + return 0; + } auto room_class = env->GetObjectClass(m_room); auto method = env->GetMethodID(room_class, "totalSize", "()J"); if (!method) @@ -1107,6 +1154,10 @@ namespace MAT_NS_BEGIN { return 0; } + if (!m_room) + { + return 0; + } auto room_class = env->GetObjectClass(m_room); auto count_id = env->GetMethodID(room_class, "getRecordCount", "(I)J"); ThrowLogic(env, "getRecordCount"); @@ -1162,6 +1213,10 @@ namespace MAT_NS_BEGIN { return false; } + if (!m_room) + { + return false; + } auto room_class = env->GetObjectClass(m_room); auto trim_id = env->GetMethodID(room_class, "trim", "(J)J"); ThrowLogic(env, "trim"); @@ -1192,6 +1247,10 @@ namespace MAT_NS_BEGIN { ConnectedEnv env(s_vm); + if (!m_room) + { + return records; + } auto room_class = env->GetObjectClass(m_room); auto method = env->GetMethodID(room_class, "getRecords", "(ZIJ)[Lcom/microsoft/applications/events/StorageRecord;"); From bf17e8a2c660e7add938bf27d23e088cf12e9332 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 16 Jun 2026 12:16:35 -0500 Subject: [PATCH 8/9] docs: note the androidx.room version the SDK is built against (#1227 follow-up) cpp-start-android.md covered Room setup but said nothing about the Room version. The GetAndReserveRecords native crash (#1227, Room-version-sensitive) and the null-guard hardening in this PR make this worth documenting: the maesdk AAR brings androidx.room transitively (pinned in maesdk/build.gradle, currently 2.8.4); since Gradle resolves one Room version app-wide, consumers should not force a version below what the SDK is built against and should prefer aligning on the bundled (or a compatible newer) version. Points at build.gradle as the source of truth so the doc doesn't drift on future bumps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/cpp-start-android.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/cpp-start-android.md b/docs/cpp-start-android.md index 0f979bcda..2281a5e31 100644 --- a/docs/cpp-start-android.md +++ b/docs/cpp-start-android.md @@ -14,6 +14,8 @@ The Gradle wrapper in ```android_build``` builds two modules, ```app``` and ```m On Android, there are two database implementations to choose from. By default (the main branch on Github), the SDK will use the Android-supported androidx.Room database package. This reduces APK size because we don't need to compile and link in a copy of SQLite in native code (SQLite is hundreds of kB per ABI of APK file size). Room does have a slight CPU performance disadvantage since database transactions cross the JNI boundary when native code uses it. If you wish to change from Room to the native SQLite implementation, you should change the two module ```build.gradle``` files (app and maesdk). In those files, you will see an argument to CMake to select Room: ```"-DUSE_ROOM=1"```. Change this to ```"-DUSE_ROOM=0``` to select the native SQLite. +When using the Room implementation, the ```maesdk``` AAR brings ```androidx.room``` as a transitive dependency, pinned in ```lib/android_build/maesdk/build.gradle``` (currently ```2.8.4```). The SDK's native (JNI) code is compiled and tested against this version and the Room-generated schema. Because Gradle resolves a single ```androidx.room``` version for the entire app, if your app (or one of its dependencies) selects a different version, the SDK's native code runs against it. **Do not force ```androidx.room``` below the version the SDK is built against**, and prefer aligning your app on the bundled version (or a compatible newer one). A significantly different Room version can change the shape of query results that cross the JNI boundary and has historically caused native crashes in record retrieval (issue #1227); the SDK now guards against null results defensively, but version alignment avoids subtle behavior differences. + The Room database implementation adds one additional initialization requirement, since it needs a pointer to the JVM and an object reference to the application context. See below (4.5) for the required call to either ```connectContext``` (in Java) or ```ConnectJVM``` (in C++) to set this up. If you are building on Windows, this helper script [build-android.cmd](../build-android.cmd) is provided to illustrate how to deploy the necessary SDK and NDK dependencies. Once you installed the necessary dependencies, you may use Android Studio IDE for local builds. See [ide.cmd](../lib/android_build/ide.cmd) that shows how to build the project from IDE. The `app` project (`maesdktest`) allows to build and run all SDK tests on either emulator or real Android device. While the tests are running, you can monitor the test results in logcat output. From 57c15f406d8c377c4eba27979b2af9f47d477e04 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 16 Jun 2026 19:57:40 -0500 Subject: [PATCH 9/9] Room: guard null JNIEnv in 5 JNI methods; skip releaseUnconsumed on null element Address the latest Copilot review (6 comments) on lib/offline/OfflineStorage_Room.cpp. ConnectedEnv null-env guards (5): DeleteByToken, ReleaseRecords, DeleteSetting, StoreSetting and GetRecords created ConnectedEnv env(s_vm) and dereferenced env->... behind only an if(!m_room) guard. ConnectedEnv::operator! can report a null JNIEnv (null s_vm / thread-attach failure), and sibling methods already guard with if(!env); added the matching early return (void/false/records) so a null env no longer crashes. releaseUnconsumed on null element: the null-array-element path broke out and fell through to releaseUnconsumed(selected, index). The Java StorageRecordDao.releaseUnconsumed (pre-existing since 2020, commit c81d46aa) reads selected[0..unconsumed-1] ignoring the offset, so on the null path it could index the null element and throw, or release the wrong rows. A sawNullElement flag now skips releaseUnconsumed on that path; the reserved records expire and are retried (no data loss), and the normal end-early path is unchanged. Validated: NDK aarch64-linux-android23 -fsyntax-only clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/offline/OfflineStorage_Room.cpp | 45 ++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/lib/offline/OfflineStorage_Room.cpp b/lib/offline/OfflineStorage_Room.cpp index a2decde1f..d052e7a9d 100644 --- a/lib/offline/OfflineStorage_Room.cpp +++ b/lib/offline/OfflineStorage_Room.cpp @@ -240,6 +240,10 @@ namespace MAT_NS_BEGIN MATSDK_THROW(std::logic_error("whereFilter not implemented")); } + if (!env) + { + return; + } if (!m_room) { return; @@ -436,6 +440,9 @@ namespace MAT_NS_BEGIN int persist_lb = static_cast(EventPersistence_Normal); int persist_ub = static_cast(EventPersistence_DoNotStoreOnDisk); + // Set if a null array element is hit below, so the early-release + // path skips releaseUnconsumed (which would index into the null). + bool sawNullElement = false; for (index = 0; index < limit; ++index) { env.pushLocalFrame(32); @@ -444,9 +451,12 @@ namespace MAT_NS_BEGIN if (!record) { // Null array element (observed with some androidx.room - // versions): pop this frame and stop. index < limit below - // releases the rest for retry rather than dereferencing - // null in GetObjectClass. + // versions): pop this frame and stop rather than + // dereferencing null in GetObjectClass. We cannot safely + // release the tail here (it contains this null and Java + // releaseUnconsumed indexes from 0), so leave the + // remaining reservations to expire and be retried. + sawNullElement = true; env.popLocalFrame(); break; } @@ -539,11 +549,14 @@ namespace MAT_NS_BEGIN if (index < limit) { // we did not consume all these events - auto release = env->GetMethodID(room_class, "releaseUnconsumed", - "([Lcom/microsoft/applications/events/StorageRecord;I)V"); - ThrowLogic(env, "releaseUnconsumed"); - env->CallVoidMethod(m_room, release, selected, static_cast(index)); - ThrowRuntime(env, "call ru"); + if (!sawNullElement) + { + auto release = env->GetMethodID(room_class, "releaseUnconsumed", + "([Lcom/microsoft/applications/events/StorageRecord;I)V"); + ThrowLogic(env, "releaseUnconsumed"); + env->CallVoidMethod(m_room, release, selected, static_cast(index)); + ThrowRuntime(env, "call ru"); + } break; // break out of the request > collected loop--end early by request } } @@ -654,6 +667,10 @@ namespace MAT_NS_BEGIN try { ConnectedEnv env(s_vm); + if (!env) + { + return; + } if (!m_room) { return; @@ -959,6 +976,10 @@ namespace MAT_NS_BEGIN try { ConnectedEnv env(s_vm); + if (!env) + { + return false; + } if (!m_room) { return false; @@ -1003,6 +1024,10 @@ namespace MAT_NS_BEGIN try { ConnectedEnv env(s_vm); + if (!env) + { + return false; + } if (!m_room) { return false; @@ -1247,6 +1272,10 @@ namespace MAT_NS_BEGIN { ConnectedEnv env(s_vm); + if (!env) + { + return records; + } if (!m_room) { return records;