Feature/additional logging#26
Conversation
…ng (3.5.14) - Android: Added INFO-level 'request mutation' log confirming token/trace header injection with missing/empty/present(len=N) state tracking - iOS: Changed all retry/failure paths from synthetic HTTP 503/499 status code responses to NSError-based failures, matching Android IOException behavior in React Native fetch()
- Enables modern React Native apps to opt into SwiftPM autolinking without breaking existing CocoaPods integrations that rely on the podspec. - Specifies approov-ios-sdk and swift-http-structured-headers dependencies.
…old start On a cold start with no cached dynamic config, calling notifyPinChangeListeners() inside initialize() triggers ApproovCertificatePinner.build() -> Approov.getPins(). getPins() calls refreshConfig() which queues a FetchConfig request and blocks on a CountDownLatch with no timeout. When 'init-fetch' was passed to Approov.initialize(), a background token fetch was queued ahead of that FetchConfig request. On a blocked network this init-fetch runs through the full retry/failover chain (~21-26s) before FetchConfig can even start, causing initialize() to block its Promise for that entire duration before resolving. Removing 'init-fetch' from the constructor means no prior request occupies the queue, so FetchConfig runs immediately and is bounded only by its own ~2s network rule timeout. The init-fetch prefetch optimisation is most relevant on initial installation; the trade-off of slightly slower first token fetch on cold start is acceptable.
notifyPinChangeListeners() still blocks promise resolution by ~2s via getPins()->refreshConfig()->CountDownLatch.await(no timeout). Low priority now that init-fetch is removed, but worth tracking for a future fix.
…atforms Port the approov-service-okhttp initialization pattern to both the Android bridge and the iOS native module: - Remove allowReinitialize / allowEnableAfterEmptyInitialization guards that introduced complex branching and were not present in okhttp. - Reset all service-layer state unconditionally at the start of every initialize() call on both Android and iOS. - Unify the two duplicated success branches (first-init and already-init) into a single code path on iOS; Android already had one path. - Release the startup sync gate (earliestNetworkRequestTime / clearEarliestNetworkRequestTime) on initialization failure as well as success so that pending requests are never held indefinitely on error. - Add null/nil guard for the config parameter on both platforms and surface it as a rejected promise. - Distinguish null comment from empty-string comment on Android (different semantics when forwarding to the platform SDK). Platform differences preserved: - Android: SDK throws IllegalArgumentException / IllegalStateException; both are caught and surfaced via promise.reject with gate cleared. - iOS: SDK returns BOOL + NSError*; NO + nil error = already initialized (success); NO + non-nil error = genuine failure, surfaced via reject.
Add a Default Behavior section before the Approov Service Mutator block, mirroring the structure used in approov-service-okhttp/USAGE.md. The table covers only the three token-fetch status groupings relevant to the network interceptor path: - SUCCESS: request proceeds with Approov-Token header - NO_NETWORK / POOR_NETWORK / MITM_DETECTED: fetch() rejects with TypeError: Network request failed (temporary failure, retry) - NO_APPROOV_SERVICE: proceeds with empty token (or status string if setUseApproovStatusIfNoToken is enabled) Includes a note explaining that UNKNOWN_URL / UNPROTECTED_URL and REJECTION statuses are intentionally excluded from the table, and a cross-reference link to the equivalent section in approov-service-okhttp.
The com.github.jengelman.gradle.plugins:shadow artifact is no longer published for 8.x. Use the maintained fork io.github.goooler.shadow:shadow which is on Maven Central and retains identical package names (so the apply plugin and ShadowJar task type references are unchanged). Also add mavenCentral() to the first buildscript repositories block since gradlePluginPortal() alone cannot resolve Maven Central artifacts.
The plugin was transferred from @johnrengelman to the GradleUp organization. The old group com.github.jengelman.gradle.plugins is deprecated and no longer published for new versions. Changes: - classpath: com.gradleup.shadow:shadow-gradle-plugin:9.4.2 - plugin ID: com.gradleup.shadow - task type: com.gradleup.shadow.tasks.ShadowJar (package renamed in 9.x) - remove unneeded mavenCentral() from first buildscript block (Gradle Plugin Portal resolves the artifact directly)
Shadow 8.x was never published under com.github.jengelman.gradle.plugins (hence the original CI failure). Shadow 9.x explicitly blocks AGP projects. The io.github.goooler.shadow 8.x fork lives on Maven Central, not the Gradle Plugin Portal, requiring an extra repository that is undesirable. 7.1.2 is the last stable release under com.github.jengelman.gradle.plugins, is present on the Gradle Plugin Portal, and works correctly with AGP.
…http
The legacy buildscript/classpath approach resolves Maven coordinates
directly. com.github.jengelman.gradle.plugins:shadow does not publish a
Maven artifact for 8.x at those coordinates.
The plugins {} DSL resolves through the Gradle Plugin Portal marker
artifact system, which correctly maps plugin ID + version to the
implementation jar. This is exactly how approov-service-okhttp declares
the same plugin at the same version.
…rement
Gradle requires all buildscript {} blocks to appear before any plugins {}
blocks. The previous commit placed the plugins {} block above the
buildscript {} block that conditionally adds the AGP classpath, causing
the 'all buildscript {} blocks must appear before any plugins {} blocks'
compile error on line 35.
Correct order: buildscript (AGP, conditional) → plugins (shadow) →
variable definitions → apply plugin.
Verified locally with Java 21 (matches CI): BUILD SUCCESSFUL in 35s,
68 tasks executed.
Per TESTING_REQUIREMENTS.md §17-18, §20 and the other service layers:
- Different-config re-initialization always resets state and forwards to
the platform SDK; the service layer is left UNINITIALIZED on failure.
- SDK failure after an empty-config bootstrap leaves the service
UNINITIALIZED (§20); the old behaviour of 'keeping it initialized but
disabled' is removed.
- Error messages are prefixed with the exception type for clarity:
'initialize IllegalArgument: <message>'
'initialize IllegalState: <message>'
- setDataHashInToken now validates its data argument before delegating to
the SDK so that null produces a deterministic IllegalArgument rejection
rather than relying on SDK-specific throw behaviour.
Test changes:
- initializeRejectsDifferentConfig: assertFalse for both state flags;
message check relaxed to contains('Illegal') to accommodate mini-SDK
throwing IllegalArgumentException vs real SDK IllegalStateException.
- initializeFailureRejectsAndKeepsLayerUninitialized: fix Mockito mock to
use nullable(String.class) for the comment parameter — any(String.class)
does not match null in Mockito 2+, so the mock never fired.
- initializeWithEmptyConfigThenSdkFailureRejectsAndBecomesUninitialized
(renamed): assertFalse after failure per requirements §20.
- initializeWithDifferentConfigResetsStateAndBecomesUninitialized
(renamed): assertFalse state flags; removed misleading 'original config
still works' network assertion.
All 100 tests pass locally (Java 21).
…S.md §17-18
Mirrors the Android test fixes from the previous commit.
The iOS ApproovService.m was already updated to reset state unconditionally
and forward every initialization call to the platform SDK. The two
different-config test functions were not updated at the same time and
still asserted the OLD behaviour (state preserved on rejection), which
contradicts the requirements document.
--- TestInitializeRejectsDifferentConfig ---
- AssertEqualObjects(@"attempt to reinitialize Approov SDK with a different config",
- rejected[@"message"],
- @"Different-config reinitialization should explain the mismatch");
- AssertTrue(isInitialized, @"Different-config reinitialization should keep the existing initialized state");
+ // Per TESTING_REQUIREMENTS §17-18: SDK message text is implementation-defined.
+ AssertTrue(!isInitialized, @"Different-config reinitialization should reset state ...");
WHY: TESTING_REQUIREMENTS.md §17 states the service layer 'always resets
its state and forwards the call to the platform SDK', and §18 states 'the
service layer becomes uninitialized'. The hardcoded expected message came
from a pre-requirements early-return guard that no longer exists. The iOS
mini-SDK surfaces the mismatch as 'initialization failed: Approov initial
configuration is malformed' (not the old hardcoded string), so the exact
message is now implementation-defined and should not be pinned.
--- TestInitializeWithDifferentConfigPreservesExistingState ---
- AssertTrue(isInitialized, @"Different-config rejection should preserve the initialized state");
- AssertEqualObjects(@(YES), enabledAfter[@"value"], @"... preserve the enabled state");
- NSDictionary *reply = FetchNetworkReply(service, TargetURL(), @{});
- AssertNotNil(HeaderValue(reply, @"Approov-Token"), @"Original config should remain functional...");
+ AssertTrue(!isInitialized, @"Different-config rejection should reset state ...");
+ AssertEqualObjects(@(NO), enabledAfter[@"value"], @"... should disable Approov");
+ // Service is now uninitialized; network request removed.
WHY: Same §17-18 reasoning. The 'original config still works' network
assertion was correct under the old preserve-state design but becomes
incorrect (and would hang/fail) when the service is left uninitialized
after the SDK rejection, since fetchWithApproov guards on isInitialized.
…STING_REQUIREMENTS.md §20 TestInitializeWithEmptyConfigThenSdkFailurePreservesBootstrap → TestInitializeWithEmptyConfigThenSdkFailureBecomesUninitialized WHAT CHANGED ------------ - // The layer stays initialized from the empty bootstrap - AssertTrue(isInitialized, - @"SDK failure after empty bootstrap should preserve the initialized state"); + // Per TESTING_REQUIREMENTS §20: service is left uninitialized after SDK failure. + AssertTrue(!isInitialized, + @"SDK failure after empty bootstrap should leave the layer uninitialized"); WHY --- TESTING_REQUIREMENTS.md §20 states: 'SDK Initialization Failure After Empty Bootstrap: If initialization with a valid non-empty config fails after the service layer was previously initialized with an empty config string, the service layer becomes uninitialized. Callers must re-initialize.' The iOS ApproovService.m was already updated to unconditionally reset all service-layer state before forwarding to the native SDK, so after any SDK failure isInitialized is NO. The test was asserting the old 'preserve bootstrap state' behaviour that pre-dates the requirements change.
…tate
Per TESTING_REQUIREMENTS.md \u00a717-18, \u00a720, and new \u00a7 'Service-Layer State Only
Updated On Success'.
THE BUG
-------
The previous implementation reset ALL service-layer state (isInitialized,
initialConfig, headers, substitution maps, ...) BEFORE calling the platform
SDK. If the SDK call then threw an exception the service layer was left
uninitialized regardless of what it was doing before:
- Protected (Approov active) \u2192 becomes UNINITIALIZED \u2192 requests forwarded
without any token or pinning \u2192 silent security regression.
- Bypass mode (empty config) \u2192 becomes UNINITIALIZED \u2192 same forwarding
path, but the effective behaviour is unchanged. Low severity, but still
wrong by the requirements.
THE FIX
-------
Move the state-reset block to INSIDE the success branch, after the SDK call
returns cleanly. The catch blocks now only release the startup sync gate (so
waiting threads are never held indefinitely) and reject the promise. No
other state is touched on failure.
Android (ApproovService.java)
- State reset (isInitialized=false \u2192 all fields cleared \u2192 isInitialized=true)
moved from before the try{} to inside the try{} after Approov.initialize()
returns without throwing.
- Catch blocks: added comments; clearEarliestNetworkRequestTime() call kept.
iOS (ApproovService.m)
- Same structural change inside @synchronized(initializerLock).
- State reset moved to after the initializationError != nil check.
- Docstring updated.
Android tests (ApproovServiceMiniSdkTest.java)
- initializeRejectsDifferentConfig:
assertFalse(isInitialized) \u2192 assertTrue(isInitialized)
assertFalse(isApproovEnabled) \u2192 assertTrue(isApproovEnabled)
- initializeWithEmptyConfigThenSdkFailureRejectsAndBecomesUninitialized
\u2192 initializeWithEmptyConfigThenSdkFailurePreservesBootstrapState
assertFalse(isInitialized) \u2192 assertTrue(isInitialized)
assertFalse(isApproovEnabled) kept (bypass mode: initialized, not enabled)
- initializeWithDifferentConfigResetsStateAndBecomesUninitialized
\u2192 initializeWithDifferentConfigPreservesExistingState
assertFalse \u2192 assertTrue for both state flags
Restored network assertion: original config still produces an Approov token.
iOS mini-SDK tests (ApproovNativeMiniSDKTests.m)
- TestInitializeRejectsDifferentConfig: !isInitialized \u2192 isInitialized
- TestInitializeWithDifferentConfig\u2026: restored state-preservation assertions
and the 'original config still works' network fetch.
iOS native tests (ApproovNativeTests.m)
- TestInitializeWithEmptyConfigThenSdkFailureBecomesUninitialized
\u2192 TestInitializeWithEmptyConfigThenSdkFailurePreservesBootstrapState
!isInitialized \u2192 isInitialized
Updated the security policy to clarify supported versions and vulnerability reporting.
Use a service-layer-specific relocation prefix to prevent duplicate class conflicts when approov-service-react-native is combined with other Approov service layers (okhttp, retrofit, volley) in the same Android application. Each service layer now uses its own relocation namespace so all can coexist without D8 dex merge failures.
The default fallback URLs (replay.ivol.workers.dev / replay-unprotected.ivol.workers.dev) were hardcoded in the workflow file, making them discoverable in the public repository. Replaced with an explicit check: the workflow now fails with a clear error if TESTING_REPLY_URL or TESTING_REPLY_URL_UNPROTECTED are not configured as GitHub organisation or repository variables.
…th a valid config
There was a problem hiding this comment.
Pull request overview
This PR improves cross-platform observability and behavioral consistency for Approov request protection in the React Native service layer, while also hardening initialization paths and Android build integration (shading + consumer ProGuard) and adding opt-in SwiftPM support.
Changes:
- Add Android INFO-level “request mutation” logging to confirm token/trace header injection, matching iOS formatting.
- Align iOS failure/retry behavior to surface
NSErrorfailures instead of synthetic HTTP responses; update iOS tests accordingly. - Harden initialization semantics across platforms; add SwiftPM
Package.swift; add Android shading + consumer ProGuard + CI endpoint hardening.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| USAGE.md | Adds default-behavior table documenting interceptor outcomes by token fetch status. |
| tests/ios/native/ApproovNativeTests.m | Updates native iOS tests for NSError-based failure behavior and init-state expectations. |
| tests/ios/native-mini-sdk/ApproovNativeMiniSDKTests.m | Adds/updates mini-SDK tests for initialization edge cases (valid→empty ignored, state preserved). |
| Package.swift | Introduces SwiftPM manifest for iOS integration. |
| package.json | Bumps version to 3.5.14. |
| ios/ApproovService.m | Reworks iOS initialization to preserve prior mode on SDK failure and clear startup gate. |
| ios/ApproovService.h | Adds nullability annotations and updates public method signatures. |
| ios/ApproovRCTInterceptor.m | Switches retry/fail paths to create NSError-based mock tasks (503/499 codes). |
| CHANGELOG.md | Documents 3.5.14 changes (logging, iOS alignment, SwiftPM, Android hardening). |
| android/src/test/java/io/approov/reactnative/ApproovServiceMiniSdkTest.java | Updates/extends Android tests for init preservation/ignore semantics and Mockito null matching. |
| android/src/main/java/io/approov/reactnative/ApproovService.java | Reworks Android initialize() semantics; adds null check for setDataHashInToken. |
| android/src/main/java/io/approov/reactnative/ApproovInterceptor.java | Adds INFO-level request mutation logging and header-state helper. |
| android/src/main/java/io/approov/reactnative/ApproovDefaultMessageSigning.java | Updates imports for relocated (shaded) BouncyCastle packages. |
| android/consumer-rules.pro | Adds consumer ProGuard rules to preserve Approov SDK/JNI interfaces. |
| android/build.gradle | Adds Shadow plugin shading setup, consumer rules wiring, and dependency changes. |
| .github/workflows/build_and_test.yml | Removes hardcoded worker URL fallbacks; requires CI variables to be set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| shadowed 'org.bouncycastle:bcprov-jdk15to18:1.84' | ||
| compileOnly files(tasks.named('shadowJar')) | ||
| implementation files(tasks.named('shadowJar')) |
| * Resets all service-layer state unconditionally before initializing the platform | ||
| * SDK. The platform SDK returns true on first initialization or false if it is |
| // capture pre-mutation header state for diagnostics | ||
| String tokenHeaderKey = approovService.getTokenHeader(); | ||
| String tokenBefore = request.header(tokenHeaderKey); | ||
|
|
| String tokenAfter = request.header(tokenHeaderKey); | ||
| String traceAfter = (traceIDHeader != null) ? request.header(traceIDHeader) : null; |
| /// @param request the requesst | ||
| /// @return the result including a modifieed request and status code and message | ||
| - (ApproovInterceptorResult *)interceptRequest:(NSURLRequest *)request; | ||
| - (ApproovInterceptorResult * _Nonnull)interceptRequest:(NSURLRequest * _Nonnull)request; |
The initialize() rewrite reset ALL service-layer state (substitution headers, exclusion URL regexes, token/binding header settings, status flags) on every successful call. The only guard was "already enabled with a valid config + empty config" — every other path, including re-initializing with the SAME config already in force, fell through to the unconditional reset. In React Native this is reached routinely: ApproovProvider calls initialize() from a useEffect, and React StrictMode double-invokes effects, Fast Refresh re-runs them, and apps may mount more than one provider. A second initialize() with the same config then silently discarded any configuration the app applied after the first call — dropping request mutations and, more seriously, exclusion URL regexes, which are security relevant. Fix: only reset the runtime configuration when the config actually changes. A same-config re-initialization (with or without a reinit/options comment) is still forwarded to the platform SDK but preserves the service-layer state. Applied identically to Android (ApproovService.java) and iOS (ApproovService.m). Tests: - Android ApproovServiceRegressionTest: same-config re-init preserves substitution/exclusion/token/binding config; different-config still resets. - iOS ApproovNativeTests: same-config re-init preserves substitution header, exclusion regex and token header. Both new tests fail without the fix.
…t unprotected requests initialize() is not a synchronized method, yet it performs a multi-field state reset that transiently sets isInitialized=false and initialConfig=null before re-committing them. The interceptor reads isInitialized, isApproovEnabled and the header/substitution/exclusion state through synchronized getters on OkHttp network threads. Because the reset wrote those non-volatile static fields with no lock (and thus no happens-before relationship with the synchronized getters), a request racing a runtime re-initialization could observe an inconsistent state — either the transient isInitialized=false/initialConfig=null window, or a re-ordered commit where isInitialized=true is visible before initialConfig is set — causing isApproovEnabled() to return false and a request that should be protected to be forwarded without an Approov token. The same window could also re-arm the startup sync gate and stall concurrent requests for up to STARTUP_SYNC_TIME_WINDOW. Fix: perform the reset and commit inside synchronized(this) so the transition is atomic relative to the synchronized getters. The platform SDK call is deliberately left outside the lock so its network work never blocks those getters. This mirrors iOS, which already performs the equivalent reset inside @synchronized(initializerLock). Verified by the existing Android unit suite (45 tests, 0 failures).
The new "request mutation" INFO log in ApproovInterceptor was emitted via android.util.Log.i directly, so it could not be suppressed through setLogLevel and logged the request URL plus token length on every single request regardless of the configured level. The iOS "task mutation" log it mirrors uses ApproovLogI, which is gated and suppressed below INFO. Fix: add a package-private, level-gated logInfo() entry point on ApproovService and route the interceptor's mutation log through it, so the log honours setLogLevel and matches the iOS behaviour instead of writing unconditionally. Verified by the existing Android unit suite (45 tests, 0 failures).
Fix initialization regressions: same-config wipe, reset race, ungated mutation log
Prevents crashes when the service layer is uninitialized or initialized in bypass mode (empty config) and a customer calls SDK methods before awaiting initialization or when the config string arrives empty. Android: - setDevKey: reject promise if not enabled - setInstallAttrsInToken: reject promise if not enabled - getMessageSignature: reject promise if not enabled - getAccountMessageSignature (static): throw ApproovException - getInstallMessageSignature (static): throw ApproovException iOS: - setDevKey: reject promise if not enabled - setInstallAttrsInToken: reject promise if not enabled - getMessageSignature: reject promise if not enabled - getDeviceID: reject promise if not enabled
| name: "approov-service-react-native", | ||
| targets: ["approov-service-react-native"] | ||
| ), |
There was a problem hiding this comment.
Resolved: Renamed the target to use underscores ('approov_service_react_native') to ensure SwiftPM module compatibility.
| ], | ||
| targets: [ | ||
| .target( | ||
| name: "approov-service-react-native", |
There was a problem hiding this comment.
Resolved: Renamed the target to use underscores ('approov_service_react_native') to ensure SwiftPM module compatibility.
| * Initializes the ApproovService with an account configuration and comment. | ||
| * Resets all service-layer state unconditionally before initializing the platform | ||
| * SDK. The platform SDK returns true on first initialization or false if it is | ||
| * already initialized with the same configuration (treated as success). Any other | ||
| * failure — such as a different-config conflict — throws and is surfaced as a | ||
| * rejected promise. |
There was a problem hiding this comment.
Resolved: Updated the Javadoc in ApproovService.java to accurately document the same-config state-preservation behavior.
…ullability crash vector, and Javadoc mismatch
Android Request Mutation Logging
Added an INFO-level request mutation log line to the OkHttp interceptor (ApproovInterceptor.java) that confirms the Approov-Token and trace ID headers were actually injected onto the outgoing request. Uses the same missing/empty/present(len=N) format as the existing iOS task mutation log, closing the observability gap where token fetch was logged but header injection was silent.
iOS Error Alignment (503 → NSError)
Changed all iOS retry and failure paths in ApproovRCTInterceptor.m from returning synthetic HTTP 503/499 status-code responses to producing NSError-based failures. This:
Aligns iOS behavior with Android, where SDK failures surface as thrown IOException / Network request failed rather than resolved HTTP responses.
Prevents customer retry logic from inadvertently looping on synthetic 503 status codes.
Ensures both platforms present Approov failures identically to React Native's fetch() API.
Initialization Hardening
Rewrote initialize() on both platforms (ApproovService.java, ApproovService.m)
SwiftPM Dual-Support
Added a root Package.swift to enable Swift Package Manager auto-linking for React Native 0.76+. Provides a native SwiftPM integration path for early adopters while maintaining full backward compatibility via the existing .podspec.
Android Build Hardening
BouncyCastle shading: Relocated org.bouncycastle → io.approov.internal.reactnative.bouncycastle via the Shadow plugin to prevent version collisions with consuming apps.
Consumer ProGuard rules: Added consumer-rules.pro to automatically preserve native SDK interfaces and JNI bindings under R8/ProGuard.
Gradle ordering: Moved buildscript block before plugins {} to satisfy Gradle ordering requirements; pinned Shadow plugin to 8.1.1.
Documentation
Added a Default Behavior table to USAGE.md covering interceptor path behavior for each fetch status.
Updated CHANGELOG.md with the 3.5.14 entry.
iOS Header Cleanup
Resolved nullability-completeness warnings in ApproovService.h by adding explicit _Nullable/_Nonnull annotations.
CI
Removed hardcoded worker URL fallbacks from the build-and-test workflow.
Testing
Android unit tests: Updated to cover different-config rejection preserving state, empty-config-after-valid-config being ignored, and null config rejection.
iOS native tests: Updated synthetic response tests to expect NSError (error code 503) instead of HTTP 503 status. Added TestInitializeWithEmptyConfigThenSdkFailurePreservesBootstrapState aligned with TESTING_REQUIREMENTS.md §20.
iOS mini-SDK tests: Added TestInitializeWithValidThenEmptyConfigIgnoresEmptyConfig and updated different-config tests per TESTING_REQUIREMENTS.md §17-18.
Resolves: approov/core-project-approov#480