Feature/zero config r8#23
Merged
Merged
Conversation
- Remove service layer re-initialization guard (allowEnableAfterEmptyInitialization, reinit comment check, same-config early return) - Service layer always resets its state then forwards non-empty config to the platform SDK; SDK returns boolean (false = already initialized) or throws - Remove null guard for config parameter; null is not a valid contract value - Fix test: different-config rejection now throws platform SDK exception, not service layer message - Fix test: null config test updated to use empty string (bypass mode) - Update REFERENCE.md: clarify 2-arg form is default (comment=null), 3-arg is the extended form; fix Kotlin comment type to String?
- Add null guard at top of initialize(): passing null config now throws IllegalArgumentException with a clear message directing callers to use "" for bypass mode; NPE is no longer the failure mode - Clarify javadoc: config does not accept null, comment accepts null
- REFERENCE.md: expand isInitialized/isApproovEnabled descriptions with bypass-mode semantics and SDK-gating note; consistent with okhttp docs - CHANGELOG.md [3.5.7]: add simplified initialize change (delegate to SDK), replace inaccurate Fixed entries with accurate null-rejection and 2-arg null-comment fix descriptions
State is now only reset after Approov.initialize() confirms success. When the SDK throws (e.g. different config), all prior operating state (isInitialized, configString, pinningInterceptor, OkHttp builder) is preserved. Per TESTING_REQUIREMENTS §17-18 (core-service-layers-testing).
Updated security policy to reflect supported versions and reporting process.
Use a service-layer-specific relocation prefix to prevent duplicate class conflicts when approov-service-retrofit is combined with other Approov service layers in the same Android application.
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
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Approov Retrofit service to be more “zero-config” for consumers by adding consumer ProGuard rules and shading BouncyCastle to avoid dependency collisions, while also simplifying/clarifying initialization semantics (including bypass mode and nullable comments).
Changes:
- Added consumer ProGuard rules and wired them into the Android library build.
- Shaded/relocated BouncyCastle and updated message-signing code to use the relocated package.
- Simplified
ApproovService.initializebehavior and updated tests/docs to reflect nullable comments and bypass/enable flows.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
REFERENCE.md |
Clarifies initialize overloads (incl. nullable comment) and semantics of isInitialized/isApproovEnabled. |
README.md |
Adds dependency/setup/usage documentation for integrating the library. |
LICENSE |
Updates copyright holder/year. |
CHANGELOG.md |
Documents 3.5.7 changes (consumer rules, shading, initialization behavior adjustments). |
approov-service/src/test/java/io/approov/service/retrofit/ApproovServiceMiniSdkTest.java |
Updates/reinforces initialization behavior expectations with Mini SDK tests. |
approov-service/src/test/java/io/approov/service/retrofit/ApproovServiceContractTest.java |
Updates contract tests for nullable comment and null-config error behavior. |
approov-service/src/main/java/io/approov/service/retrofit/ApproovService.java |
Implements new initialization flow and minor formatting/doc adjustments. |
approov-service/src/main/java/io/approov/service/retrofit/ApproovDefaultMessageSigning.java |
Switches BouncyCastle imports to the relocated/shaded namespace. |
approov-service/pom.xml |
Removes the transitive BouncyCastle runtime dependency entry. |
approov-service/consumer-rules.pro |
Introduces consumer ProGuard rules to preserve Approov SDK/JNI interfaces. |
approov-service/build.gradle |
Adds Shadow plugin + shading task, consumer ProGuard wiring, and dependencies for shaded jar. |
.vscode/settings.json |
Adds VS Code Java build configuration setting. |
.github/workflows/build_and_test.yml |
Requires worker endpoints via GitHub variables (no hardcoded fallback) and updates CI behavior accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ocument empty-token prefix-only behaviour - ApproovTokenInterceptor: add null+empty guard before writing substituted header values and query parameter values. When getSecureString() returns null or empty, the original placeholder is preserved in place per TESTING_REQUIREMENTS §2 Missing Artifacts Fallback. - REFERENCE.md: document token header emission behaviour when no real token is available, including prefix-only case when useApproovStatusIfNoToken=false.
…, CI comments - README.md: fix gradle.build→build.gradle (C1); add Groovy DSL variant for dependency snippet (C2); fix missing word in line 70 (C3); fix missing verb in line 89 (C4); fix crytographically→cryptographically (C5) - CHANGELOG.md: fix BC relocated package name io.approov.internal.bouncycastle →io.approov.internal.retrofit.bouncycastle (C6) - SECURITY.md: fix supported version table (<3.3→<3.5.0/3.4.0); fix recieve→receive typo (merged from feature/3.6.0 PR#21) - consumer-rules.pro: fix comment to accurately describe what is retained (C9) - .github/workflows/build_and_test.yml: remove stale fallback/precedence comment; script hard-fails on missing variables, no fallback (C10) - ApproovService.java: fix succesfull→successful in getLastARC Javadoc (C11)
This was referenced Jun 10, 2026
Closed
…, SECURITY, CI CHANGELOG.md: - 'cryptography bounds' -> 'cryptography bindings' (B2) - Clarify initialize state-reset wording: state reset only after SDK confirms success; failed call preserves existing state (B3) REFERENCE.md: - isInitialized(): prior state is preserved on failure, not flipped to false (C1) - setUseApproovStatusIfNoToken: covers both empty-token-on-success AND failure statuses allowed to proceed by the active mutator (C2) ApproovService.java: - initialize() Javadoc: 'empty for no comment' -> 'or null for no comment' (D1) SECURITY.md: - 'less functionalities' -> 'less functionality' (G1) - 'vulnerabilities to this project' -> 'in this project'; fix run-on sentence (G2) .github/workflows/build_and_test.yml: - Remove reference to non-existent CONTRIBUTING.md in CI error message (F2) Not fixed: - build.gradle shadow task (H1): works in CI; risk of regression without testing - initialize state reset on same-config (I1): intentional per TESTING_REQUIREMENTS §1
Mirror the parity improvement made on the Alamofire service layer (approov-service-alamofire c10e11a) for cross-platform consistency. A successful initialize — including the benign same-config no-op — resets all service-layer configuration to defaults, as required by the shared TESTING_REQUIREMENTS spec. This was undocumented and silent. Now: - Log.w when a re-init discards previously applied configuration. - Javadoc on initialize(context, config, comment) documents the reset contract and the full prior-state/config behavior matrix. - REFERENCE.md gains a "Re-initialization and state reset" subsection. No behavior change beyond the added warning log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
testInitializeWithEmptyConfigBuildsPlainClient and testInitializeWithEmptyConfigCanLaterEnableApproov asserted that an empty config applied to an already-protected service layer downgrades it to bypass mode. That is the pre-rework behaviour; the new initialize correctly refuses to downgrade a protected layer on a later empty config (TESTING_REQUIREMENTS §1 "Empty Configuration after Valid Configuration"), which is now covered by testInitializeWithValidThenEmptyConfigIgnoresEmptyConfig. These two tests are meant to verify genuine bypass-from-scratch behaviour, so reset the service layer to an uninitialized state (resetApproovServiceState) before the empty-config initialize, restoring their original intent without duplicating the new "stays protected" test. Full mini-SDK suite: 20/20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR hardens the Retrofit service layer with zero-config R8 compatibility, BouncyCastle relocation, initialization robustness improvements, and documentation updates.
R8 / ProGuard Hardening
consumer-rules.prowith keep rules for Approov SDK, BouncyCastle, and message signing classes. These are automatically applied to consuming apps viaconsumerProguardFiles, eliminating the need for manual ProGuard configuration.io.approov.internal.retrofit.bouncycastleusing the Shadow Gradle plugin to avoid classpath conflicts with apps that bundle their own BouncyCastle version.Initialization Robustness
initialize(null)now throwsIllegalArgumentExceptionwith a clear message instead of silently failing.initializenow accepts acommentparameter forwarded to the native SDK foroptions:andreinitflows, documented in REFERENCE.md.Documentation
initializecomment options, reinitialization semantics, and null/empty config behavior.CI
Files Changed
ApproovService.javaconsumer-rules.probuild.gradlepom.xmlApproovDefaultMessageSigning.javaREADME.mdREFERENCE.mdCHANGELOG.mdApproovServiceContractTest.javaApproovServiceMiniSdkTest.javaRelated: approov/core-project-approov#558