Cross-platform App Hardening (DexGuard-class), Enterprise-gated - #5527
Cross-platform App Hardening (DexGuard-class), Enterprise-gated#5527shai-almog wants to merge 96 commits into
Conversation
Adds a single hardening layer that renames classes/methods/fields, encrypts string constants and obfuscates control flow across every port (Android, iOS/ ParparVM, JavaScript, native desktop) from one bytecode transform, integrated with Crash Protection so obfuscated stack traces are still symbolicated. Engine (new maven/cn1-hardening, run as a forked process so it is single-sourced with the build daemon and carries its own ProGuard/ASM): demux the fat jar, rename with ProGuard using a prefixed dictionary that avoids the ParparVM NativeSymbolIndex culler pathology, encrypt LDC literals and static-final ConstantValue strings with a per-class decoder, opaque-predicate control flow on safe platforms, ParparVM mangle-collision guard, CheckClassAdapter verification, and a cross-platform mapping. Android keeps R8 as its sole renamer. Symbolication (new maven/cn1-retrace): ProGuard mapping parse/chain plus the ParparVM trace-string parser that java.lang.Throwable.getStackTrace() now mirrors on device, and a local retrace CLI. Crash payload gains rawStack/traceFormat/ mappingId/hardenLevel; PiiScrubber.scrubRawStack; cause-chain capture. Surface/entitlement: harden.* build hints, HardeningPreflight (fail the build on local/source targets, invalid level, on-device-debug), Executor.hardenSourceJar/ runBuild wiring, a read-only Hardening status API, and the App-Hardening developer guide chapter. Also fixes the invalid build_key literal, the BuildHintEditor grouped-Select values lookup, and the "obfuscates by default" overclaim in the security chapter. Tests: 25 unit tests across the two modules and the crash payload (full ProGuard round-trip, string round-trip + plaintext-absence, control-flow verification, mapping retrace, trace-format detection, pre-flight truth table). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae40837827
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
There was a problem hiding this comment.
Pull request overview
Introduces an Enterprise-gated, cross-platform “App Hardening” pipeline (bytecode-level renaming + string encryption + optional control-flow obfuscation) and integrates it with Crash Protection so hardened builds can still be symbolicated using a retained mapping.
Changes:
- Adds new Maven modules:
cn1-hardening(engine) andcn1-retrace(mapping/trace parsing + retrace CLI). - Integrates hardening into the Maven plugin build flow (preflight checks + engine invocation + platform IDs), and updates Crash Protection payload schema to include
rawStack/traceFormat/mappingId/hardenLevel. - Improves ParparVM
Throwable.getStackTrace()by parsing the native stack string into structured frames.
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/JavaAPI/src/java/lang/Throwable.java | Parse ParparVM stack text into StackTraceElements |
| tests/core/test/com/codename1/crash/CrashReportPayloadTest.java | Tests for traceFormat + hardening fields JSON |
| Ports/JavaSE/src/com/codename1/impl/javase/BuildHintSchemaDefaults.java | Adds hardening build-hint schema defaults |
| Ports/JavaSE/src/com/codename1/impl/javase/BuildHintEditor.java | Fix grouped Select .values resolution |
| maven/pom.xml | Adds hardening/retrace modules; bumps ProGuard |
| maven/codenameone-maven-plugin/src/test/java/com/codename1/maven/HardeningPreflightTest.java | Unit tests for hardening preflight matrix |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/HardeningPreflight.java | Client-side preflight validation logic |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/CN1BuildMojo.java | Invoke preflight; route local builds via runBuild |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/WindowsNativeBuilder.java | Provides hardening platform id |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/LinuxNativeBuilder.java | Provides hardening platform id |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/JavaScriptBuilder.java | Provides hardening platform id |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/IPhoneBuilder.java | Provides hardening platform id |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/Executor.java | Fork/execute hardening engine; mappingId/buildKey handling |
| maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/AndroidGradleBuilder.java | Android: disable engine renaming; mappingId + encoded build_key |
| maven/codenameone-maven-plugin/pom.xml | Embeds shaded hardening jar into plugin resources |
| maven/cn1-retrace/src/test/java/com/codename1/retrace/ParparVmTraceParserTest.java | Golden tests for ParparVM trace parser |
| maven/cn1-retrace/src/test/java/com/codename1/retrace/MappingFileTest.java | Tests for mapping parse + retrace + chaining |
| maven/cn1-retrace/src/main/java/com/codename1/retrace/RetraceMain.java | Standalone retrace CLI entrypoint |
| maven/cn1-retrace/src/main/java/com/codename1/retrace/ParparVmTraceParser.java | Parses ParparVM text traces; rejects JS stacks |
| maven/cn1-retrace/src/main/java/com/codename1/retrace/MappingFile.java | Parses/inverts ProGuard mapping for retrace |
| maven/cn1-retrace/src/main/java/com/codename1/retrace/MappingChain.java | Applies multiple mappings in order |
| maven/cn1-retrace/src/main/java/com/codename1/retrace/Frame.java | Frame DTO for retrace pipeline |
| maven/cn1-retrace/pom.xml | Module build + shaded standalone jar |
| maven/cn1-hardening/src/test/java/com/codename1/hardening/StringEncryptTransformTest.java | End-to-end string encryption tests |
| maven/cn1-hardening/src/test/java/com/codename1/hardening/fixture/Secrets.java | Fixture class for encryption tests |
| maven/cn1-hardening/src/test/java/com/codename1/hardening/fixture/Helper.java | Fixture class for rename tests |
| maven/cn1-hardening/src/test/java/com/codename1/hardening/ControlFlowTransformTest.java | Control-flow guard verification tests |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/StringEncryptTransform.java | ASM transform: encrypt LDC + ConstantValue strings |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/ProGuardRunner.java | Programmatic ProGuard rename runner |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/OutputVerifier.java | CheckClassAdapter verification gate |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/MappingWriter.java | Mapping header + mappingId computation |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/MangleCollisionCheck.java | Guards ParparVM symbol mangle collisions |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/Main.java | Engine CLI front-end + exit codes |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/JarDemuxer.java | Split/rebuild fat jar around class entries |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/InputJarKeepScanner.java | ASM scan for reflective keep rules |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningResult.java | Engine result model |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningRequest.java | Engine request model |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningProfile.java | Hardening levels + defaults |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningException.java | Engine exception type |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningEngine.java | Pipeline orchestration + safety gates |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningConfig.java | Parses harden.* hints into config |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/ControlFlowTransform.java | ASM opaque-predicate entry guards |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/Cn1NameFactory.java | zq-prefixed dictionary generator |
| maven/cn1-hardening/src/main/java/com/codename1/hardening/BuiltinKeepRules.java | Tier-1 keep rules + shared R8 rules |
| maven/cn1-hardening/pom.xml | Engine module deps + shaded standalone jar |
| docs/developer-guide/security.asciidoc | Correct obfuscation claims; reference hardening |
| docs/developer-guide/developer-guide.asciidoc | Include App-Hardening chapter |
| docs/developer-guide/Crash-Protection.asciidoc | Document new crash payload fields + hardening flow |
| docs/developer-guide/App-Hardening.asciidoc | New hardening chapter |
| CodenameOne/src/com/codename1/security/hardening/package-info.java | Package docs for Hardening API |
| CodenameOne/src/com/codename1/security/hardening/Hardening.java | Read-only hardening status API |
| CodenameOne/src/com/codename1/crash/PiiScrubber.java | Add scrubRawStack hook |
| CodenameOne/src/com/codename1/crash/CrashReportPayload.java | Add rawStack/traceFormat/mappingId/hardenLevel |
| CodenameOne/src/com/codename1/crash/CrashProtection.java | Capture/scrub rawStack; include in payload |
Suppressed comments (2)
maven/cn1-hardening/src/main/java/com/codename1/hardening/HardeningConfig.java:99
- Any unrecognized harden.strings value currently falls into the final else branch and enables maximum string encryption. This can turn on expensive/behaviour-changing encryption accidentally (e.g. a typo), so the parser should only treat explicit "all"/"true"/"2"/"3" as 'all' and otherwise fall back to the level defaults (or fail).
} else {
// "all", "true", "2", "3"
encConst = true;
encAll = true;
}
docs/developer-guide/Crash-Protection.asciidoc:93
- This paragraph states that on ParparVM ports the Java trace arrives only as rawStack and is parsed server-side. With Throwable.getStackTrace() now parsing the ParparVM stack string into structured frames on-device, the server may receive both; consider updating this sentence to reflect that rawStack is still
parparvm-textbut frames may be structured too (and rawStack may primarily be for the cause-chain / verbatim trace).
When a build is hardened (see <<app-hardening>>), the build server retains the cross-platform obfuscation mapping and symbolicates incoming reports against it, so a hardened build's crashes still land as readable, correctly-lined issues. Two things follow from how the mapping is retained: a report whose mapping has aged out of retention can no longer be retraced, and a locally hardened build -- whose mapping never reached the server -- can't be symbolicated at all. On the ParparVM ports (iOS, tvOS, watchOS, mac-native, Windows, Linux) the Java trace arrives as `rawStack` in the `parparvm-text` format and is parsed server-side; on the JavaScript port it arrives as a JavaScript engine stack (`js-error`) and is symbolicated best-effort through the source map.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Cloudflare Preview
|
- Complete GPLv2+Classpath header on the 4 files the copyright gate flagged (BuildHintEditor had none; BuildHintSchemaDefaults + the two new tests were short). - Convert Hardening.java/package-info.java to /// markdown comments (core src gate). - Declare cn1-hardening:standalone as a runtime-scope plugin dependency so the reactor builds the engine before the plugin embeds it (fixes the antrun copy failing in CI); fix an illegal -- inside the new XML comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ProGuard 7.3.2 cannot read class files newer than JDK 20 (it fails on the JDK's own module classes), so the renamer must run on JDK 8-20 -- the cloud daemon forks the engine on JDK 17. The engine now fails with a clear message instead of a cryptic ProGuard error when renaming is requested on a too-new JVM, and the ProGuard-dependent tests skip (JUnit assumption) on JDK 21+ so the PR CI JDK-21 leg stays green. String encryption and control-flow tests have no such limit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc8822afde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- FrameClassWriter: COMPUTE_FRAMES resolved common superclasses through the engine's own classloader, which lacks the app/library classes when run as a forked jar, so any class with a merge between application types aborted hardening. Resolve the hierarchy from a classloader over the (renamed) input classes plus the library jars, falling back to Object. Threaded through the string-encryption and control-flow transforms; unit-tested. (Codex P1) - Pass the FULLY QUALIFIED main class to the keep rules: getMainClass() is the simple name, so a bare value kept a default-package class and let ProGuard rename the real application class out from under the generated stub. Fixed in both the plugin and daemon config writers. (Codex P1) - The reactor dependency forcing cn1-hardening to build before the plugin (so the engine jar exists for the embed step) already landed in the prior commit. (Codex P1) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 57 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Ports/JavaSE/src/com/codename1/impl/javase/BuildHintEditor.java:206
- Select-hint value parsing uses the last character of the values string as the delimiter (
charAt(len-1)), which breaks all of the comma-separated.valuesentries coming fromBuildHintSchemaDefaults(e.g.off,standard,aggressive,paranoidwill split ond). This prevents the new Hardening Select hints (and existing ones likenativeTheme) from populating correctly.
String separator = ""+valuesString.charAt(valuesString.length()-1);
ArrayList<String> values = new ArrayList<String>();
values.add("");
for (String value : valuesString.split(separator)) {
docs/developer-guide/Crash-Protection.asciidoc:86
- This section says ParparVM ports rely on
rawStackbecausegetStackTrace()yields only a formatted string, but this PR updatesvm/JavaAPI/java/lang/Throwable.getStackTrace()to parse the ParparVM text stack into structuredStackTraceElement[]. The docs should be updated to reflect that structured frames are now available on ParparVM (andrawStackis supplemental / for the full rendered text and cause chain).
- `frames[]` -- class / method / file / line / `native` flag per frame
- `rawStack` -- the pre-rendered Java stack (via `printStackTrace`, including the cause chain). On the ParparVM ports this is the readable Java trace, since `getStackTrace()` there yields a formatted string rather than structured frames
- `traceFormat` -- how the server should read `rawStack`: `structured`, `parparvm-text`, `js-error`, or `none`. Derived, never guessed
maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/Executor.java:2457
- The generic
catch (Exception e)wraps the error without the original cause, which makes diagnosing hardening failures much harder (stack trace is lost). Preserve the cause in theBuildException.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efa163b074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 57 changed files in this pull request and generated no new comments.
Suppressed comments (2)
vm/JavaAPI/src/java/lang/Throwable.java:144
- setStackTrace() updates parsedStack, but printStackTrace() prints the pre-rendered "stack" string field. Because setStackTrace() doesn't update "stack", callers that set a custom stack trace will still see the old/empty stack printed (and CrashProtection.safeRawStack() relies on printStackTrace()). Rebuild the "stack" string from the provided StackTraceElement[] so printStackTrace() reflects the updated trace.
CodenameOne/src/com/codename1/crash/CrashReportPayload.java:120 - CrashReportPayload documents hardenLevel as one of "off"/"standard"/"aggressive"/"paranoid", but it currently defaults to the empty string when the property isn't set. This makes the payload ambiguous for unhardened builds and diverges from Hardening.getLevel() which defaults to "off".
this.osVersion = d.getProperty("OSVer", "");
this.mappingId = d.getProperty("cn1.mappingId", "");
this.hardenLevel = d.getProperty("cn1.hardenLevel", "");
Locale loc = Locale.getDefault();
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Codex P1/P2: - harden.keep: split on newlines only (a ';' is legal inside a rule body). - Keep SourceFile,LineNumberTable so ParparVM/native traces keep real line numbers for retrace. - Honor constants-vs-all string mode: 'constants' encrypts only values declared as static-final String constants (and javac's inlined copies), 'all' encrypts every literal. - Propagate cn1.mappingId/cn1.hardened/cn1.hardenLevel into the request before stub generation; Android stub now stamps them (Hardening.isHardened(), crash report mappingId/level). - Supply the compile/platform classpath to ProGuard as library jars so an app method overriding a framework method is not renamed apart from its superclass. - Append harden.keep + the name-bound PropertyBusinessObject keep to Android's R8 config (Android keeps R8 as sole renamer). Build fixes: - CrashProtection.safeRawStack: build the raw stack with StringBuilder instead of java.io.PrintWriter, which the core's CLDC11 bootclasspath (ANT build) lacks. - Embed the engine jar via maven-dependency-plugin:copy (resolves the standalone artifact from the reactor/repo) so partial plugin-only CI builds no longer fail copying from an unbuilt sibling target/. - Keep the test resource bytes ASCII (explicit byte[] rather than a non-ASCII literal). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 605f722c41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…, Select delimiters) - OutputVerifier: pass the input/library hierarchy classloader to CheckClassAdapter.verify so the final verification pass resolves application types instead of loading them from the engine's classpath (a class with a merge between app types would otherwise fail verification). (P1) - Keep every class named by a META-INF/services/* descriptor (the service interface and each provider), since the descriptors are copied verbatim and ServiceLoader would break if they were renamed; regression-tested. (P1) - Terminate the hardening Select .values lists with their delimiter, which BuildHintEditor reads as the last character, so the simulator shows the real options instead of splitting on a letter. (P2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Compared 12 screenshots: 12 matched. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fffb4c59f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 57 changed files in this pull request and generated no new comments.
Suppressed comments (5)
maven/cn1-hardening/src/main/java/com/codename1/hardening/JarDemuxer.java:120
rebuild()doesn’t close itsZipOutputStream. Even with an explicitfinish(), closing the stream is the reliable way to release the deflater/native resources and ensure the central directory is flushed.
FileOutputStream fo = new FileOutputStream(outJar);
try {
ZipOutputStream zos = new ZipOutputStream(new BufferedOutputStream(fo));
for (Map.Entry<String, byte[]> e : classesByInternalName.entrySet()) {
ZipEntry entry = new ZipEntry(e.getKey() + ".class");
maven/cn1-hardening/src/main/java/com/codename1/hardening/JarDemuxer.java:145
readClasses()creates aZipInputStreambut never closes it. Closing only the underlyingFileInputStreamworks most of the time, but it’s safer and clearer to close the ZIP stream explicitly (try-with-resources).
FileInputStream fi = new FileInputStream(jar);
try {
ZipInputStream zis = new ZipInputStream(fi);
ZipEntry entry;
while ((entry = zis.getNextEntry()) != null) {
Ports/JavaSE/src/com/codename1/impl/javase/BuildHintEditor.java:201
propName.indexOf("}}.")is assumed to be present here. If it isn’t (unexpected/third-party schema key), the currentsubstring(... + 3)call will produce an incorrect key (and can throw if the key is shorter), andvaluesString.charAt(valuesString.length()-1)will also throw on an empty values string. Consider guarding the index/length and making the value-splitting robust (comma-separated lists in this repo don’t end with a delimiter).
String valuesKey = propName.substring(0, propName.indexOf("}}.")+3) + "values";
String valuesString = System.getProperty(valuesKey);
if (valuesString == null) {
valuesString = System.getProperty("codename1.arg.{{ "+model.name+" }}.values");
}
CodenameOne/src/com/codename1/crash/CrashReportPayload.java:147
deriveTraceFormat()can classify a raw stack asparparvm-texteven if the frame body contains spaces. That diverges from the on-device ParparVM parser logic (which rejects frames containing spaces) and could send the server down the wrong parse path for some JS engine stack formats.
int at = rawStack.indexOf(" at ");
if (at >= 0) {
int lineEnd = rawStack.indexOf('\n', at);
String body = lineEnd < 0 ? rawStack.substring(at + 7) : rawStack.substring(at + 7, lineEnd);
if (body.indexOf('(') < 0 && body.indexOf('/') < 0 && body.indexOf('@') < 0) {
maven/cn1-hardening/src/main/java/com/codename1/hardening/JarDemuxer.java:103
split()createsZipInputStream/ZipOutputStreambut never closes them. Relying on only closing the underlyingFileInputStream/FileOutputStreamcan leak native resources and (for the output) risks incomplete ZIP finalization on some JVMs/filesystems. Use try-with-resources to ensure ZIP streams are closed.
This issue also appears in the following locations of the same file:
- line 116
- line 141
ZipInputStream zis = new ZipInputStream(fi);
FileOutputStream fo = new FileOutputStream(classesJarOut);
try {
ZipOutputStream zos = new ZipOutputStream(new BufferedOutputStream(fo));
ZipEntry entry;
… form Codex is right that shape validation alone cannot make the fn@url:line:col form safe: Firefox/Safari emit those frames WITHOUT indentation, so a message continuation that reproduces the grammar exactly (user@https://host/app.js:1:123456) is indistinguishable from a real frame -- the earlier source-shape and URL-path checks all fall to a crafted message that simply includes a URL. Since there is no reliable discriminator, isFrameLine no longer recognizes the '@' form: an unindented '@' line is treated as a message and fully scrubbed, so its numeric tail can never bypass digit masking or a scrubMessage override. The only cost is that a Firefox/Safari raw-stack frame's column is masked in the uploaded text; the structured frames still carry the coordinate for symbolication, and the INDENTED V8 'at ...:line:col' form (the common minified-bundle case) still preserves it because indentation distinguishes it from a wrapped message. Removed the now-dead atSignFrame/endsWithLineColumn helpers and consolidated the @-form tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2844caf2c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex's meta-point is correct and applies to every form, not just the '@' one: printStackTrace writes the exception MESSAGE verbatim, and a message can contain an embedded, indented, frame-shaped line (e.g. code that folds another stack trace into a message text). Such a line is indistinguishable from a real frame by ANY indentation or shape check, so every heuristic I added (indentation, V8 label grammar, accessor-alias validation) still fell to a message that reproduces the grammar exactly, letting a planted :line:column tail bypass digit masking. scrubRawStack now routes EVERY line through scrubMessage -- no line is treated as a frame whose coordinate is preserved. scrubMessage masks only 6+ digit runs, so ordinary short line numbers stay readable while a large minified-bundle column (or a long id planted as a fake column) is masked; precise coordinates for symbolication come from the structured frames, which are real StackTraceElements rather than parsed text. The app's scrubFrame override is still applied so a redacted synthetic method name does not resurface. Removed the now-dead frame-detection helpers (isFrameLine/atFrame/isV8FrameIdentity/isParenLocation/ isDottedOrUrlIdentity/endsWithColonNumber/startsWithWhitespace) and updated the tests to the uniform behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 875477d84d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ry for inherited member scopes - applyFrameOverride treated a null scrubFrame() result as 'no change' and returned the original line, so an app that returns null to REMOVE a sensitive synthetic method name had it restored in rawStack even though the structured frame renders it empty (CrashReportPayload.Frame maps null -> ""). It now renders the method empty on null, matching the structured path. Test scrubFrameOverrideReturningNullRemovesTheMethodFromRawStack. - The round-75 dictionary sizing used the per-class member count, but ProGuard cannot give two same-descriptor methods in one inheritance hierarchy the same obfuscated name without an accidental override, so same-descriptor methods accumulate ACROSS a hierarchy. Five classes each with 12000 ()V methods need 60000 collision-free names, but the per-class max stayed at the 50000 floor and ProGuard fell back to short names, reviving the ParparVM native-scan pathology. maxMemberNamingScope now sizes from the largest field-count-per-class AND the count of methods sharing a descriptor across the whole jar (a safe upper bound on the per-hierarchy scope). Test memberNamingScopeAccumulatesSameDescriptorMethodsAcrossClasses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcbaf7f7d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…be loaded FrameClassWriter.getCommonSuperClass fell straight back to java/lang/Object whenever loading a type threw. But when two application types A and B share a superclass Base that is supplied only by the target platform and is absent from cn1.libraryjars, loading A or B fails (their super can't be linked), so a merge of A and B was recorded as Object. Object is not assignable to Base, so if the merged value is then used where Base is expected the generated StackMapTable fails verification and hardening rejects an otherwise valid app. On a load failure the resolver now reads the super_class name from the class BYTES -- which does not require the (absent) supertype to be loadable -- walking A's ancestor chain and then B's until they meet, so a real common supertype like Base is found; it collapses to Object only when the bytes themselves cannot be read. New test resolvesSharedBaseFromBytesWhenBaseCannotBeLoaded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c3ae44373
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The byte-based fallback added last commit walked only super_class, so when a control-flow join merged an interface-typed value with a class that implements that interface but extends an absent (target-platform) superclass, it resolved to Object even though the class bytes declare the interface -- a following invokeinterface then got an incompatible stack-map type and hardening rejected the valid class. The byte-based resolution now mirrors the load-based logic exactly, using an assignability check that walks BOTH super_class and interfaces[]: if one type is a supertype (superclass or implemented/extended interface) of the other it wins; an unrelated interface merge is Object (as the verifier treats interfaces); two unrelated classes resolve to the first shared super_class. New test resolvesImplementedInterfaceFromBytesWhenSuperclass IsAbsent (C implements app/I, extends an absent app/Base -> merge with I resolves to app/I). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4f35e86ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…esolve a type OutputVerifier runs CheckClassAdapter.verify, whose data-flow SimpleVerifier LOADS types to resolve the hierarchy. When an application class extends a superclass supplied only by the target platform and absent from cn1.libraryjars, loading it throws (a TypeNotPresentException/NoClassDefFoundError) and the verifier rejected the class -- even though FrameClassWriter now computes its frames correctly from the bytes and the target JVM verifies it on-device. That is a missing-type failure, not a bytecode defect. verify() now detects an unresolved-type failure (scanning the cause chain for TypeNotPresentException / ClassNotFoundException / NoClassDefFoundError) and falls back to STRUCTURAL verification for that class (CheckClassAdapter with data-flow off, which needs no hierarchy), so a structurally invalid transform is still caught while a valid class with a target-only supertype is no longer wrongly rejected. A real verification error or a class with a resolvable hierarchy is unaffected. New OutputVerifierTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34cb4013b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MappingWriter.injectSourceFiles writes the sourceFile value JSON-escaped (jsonEscape backslash-escapes " and \), but parseSourceFileMetadata read it by stopping at the first RAW quote and never decoding escapes. A source filename containing a quote or a backslash -- a Unix path can -- was therefore truncated at the escaped quote or left with literal backslashes, so the retraced frame pointed at a non-existent file and source links broke. The parser now hand-scans the JSON string, treating a backslash as an escape (the next character is literal, covering the " and \ the writer emits) and stopping only at an UNescaped quote. New test decodesJsonEscapesInSourceFileMetadata (weird\"name\\.kt -> weird"name\.kt). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20ef9be386
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stage parparvm-java-api.jar as a library jar for the ParparVM-to-JS build so the hardening engine excludes its literals, exactly as on native ParparVM-C targets. hardeningPlatform() is "javascript" for that builder, which the base class does not treat as ParparVM-C, so JavaScriptBuilder opts in via a new stagesParparVMRuntime(request) hook. Without this, an encrypted app literal would compare != to a runtime-returned copy in the hardened browser build. Also document in FrameClassWriter that ASM's getCommonSuperClass only ever receives element internal names (never array descriptors) for same-dimension reference-array merges -- and is not called at all for array-vs-scalar or different-dimension merges -- so the byte-based scalar path already resolves a shared-but-unloadable Base to Base[], and an array-descriptor branch would be unreachable. Verified empirically against ASM 9.8. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be80678a08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
countShortLiterals scanned only method LDCs, but encryptStaticFinalStrings skips a short (length <= 2) static-final String via shouldEncryptLiteral -> shouldEncrypt, leaving its ConstantValue plaintext in the class file (and in ParparVM's C pool). Such a value was left uncounted, so an strings:all build could advertise full coverage while a short constant leaked. Include the static-final field channel in the count (distinct by value, dedup'd with the LDC channel), mirroring countOversizedLiterals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e4833c15c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cd67a8530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…M-wide CN1BuildMojo stashed the pre-flight force-off and compile-classpath decisions in process-wide System properties, which race under concurrent module builds (mvn -T): another platform's CN1BuildMojo could clear cn1.harden.forceOff between this build setting it and Executor.hardenSourceJar() reading it, so a local/source build that took harden.allowUnhardenedLocalBuild would run hardening anyway and produce the orphaned mapping the pre-flight prevents. Hold the decisions on the Mojo instance (each reactor module runs its own instance, so they are naturally per-build) and inject them into that build's BuildRequest via applyHardeningRequestArgs. Executor now reads cn1.harden.forceOff from the request first, falling back to the System property only for callers not yet migrated (mirroring cn1.hardening.libraryJars). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The byte-based getCommonSuperClass fallback resolves a shared base only when each app type's superclass chain is readable. When two joined types extend different target-only intermediates whose common base is ALSO absent (A extends PlatformA, B extends PlatformB, both extend an absent Base), the chain breaks at the missing intermediate and the merge collapsed to Object. Because the transforms discard the original frames and run COMPUTE_FRAMES, that Object could be weaker than the type the target expects, and the app would fail on-device verification. FrameClassWriter now flags such a collapse (isHierarchyIncomplete: it distinguishes a genuine Object merge, whose chains both reach Object through readable bytes, from one broken by an absent link). The string and control-flow transforms detect the flag and ship that class UNHARDENED -- its original, javac-computed frames already encode the precise type. The string transform also excludes all of that class's literals jar-wide, since they stay plaintext and a value encrypted+interned elsewhere must not compare != to this class's copy on ParparVM. The count is disclosed as a build warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63a4fe71d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When the incomplete-hierarchy fallback ships a class unhardened, the guard/ encryption counts had already been incremented for the attempted work. The engine aggregates those and uses them to decide that a transform ran, so with rename off and every guardable/encryptable class taking the fallback, the output was byte-unmodified yet stamped cn1.hardened=true. Reset guardedMethods (and oversizedMethods) in ControlFlowTransform and encryptedCount in StringEncryptTransform on the discard path, so only emitted transforms count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MappingWriter.jsonEscape only escaped quote and backslash, so a SourceFile containing a newline, tab or other control character was emitted literally -- a raw newline split the single-line metadata comment and a raw control char is invalid JSON, either of which stopped MappingFile.parseSourceFileMetadata from recovering the filename (retrace then fell back to a synthesized path). This is reachable for a Kotlin or package-private Java class in an unusually named Unix file. Escape b/f/n/r/t and other control chars (as the 4-hex-digit form), and teach the reader to decode those escapes back. Round-trip tested both sides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 799372c730
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
parseSourceFileMetadata trim()'d the decoded fileName, so a Unix source file whose name has leading/trailing whitespace (e.g. " Screen.kt") retraced to a different, nonexistent filename and broke source links. The closing quote bounds the JSON string exactly, so return the decoded value verbatim and treat only a genuinely empty value as absent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 374c2168b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A HealthBackgroundListener is reconstructed after a process restart by its persisted class name: HealthStore writes getClass().getName() to Preferences, and the platform builder scans the hardened jar to generate a factory mapping that name back to a constructor. The engine's keep set omitted it, so an engine-renamed Apple/native build renamed the implementor; because the default mapping seed changes between builds, an updated app's regenerated factory no longer recognized the previously persisted name and resolveBackgroundListener() silently returned nothing, deferring background health delivery. Keep the implementors, exactly as the location/background-callback and Login rules do. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 364d1101ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // So there is no rename-vs-source mismatch to guard; a source-text keep scanner | ||
| // would be dead code. Revisit only if a builder starts compiling unzip's sourceDir | ||
| // against an engine-renamed classpath. | ||
| nonClass.put(name, data); |
There was a problem hiding this comment.
When an engine-renamed class calls Class.getResourceAsStream("config.properties"), the runtime prepends the class's renamed package, but this code retains the resource under its original package path. This affects valid application and dependency resources on iOS/native/JavaSE targets where the engine renames packages, causing the lookup to return null; resource filenames need to be adapted using the class mapping or the relevant classes/packages must be kept.
Useful? React with 👍 / 👎.
| if (isInterfaceFromBytes(type1) || isInterfaceFromBytes(type2)) { | ||
| return "java/lang/Object"; |
There was a problem hiding this comment.
When a merge joins a known interface I with a class C whose bytes declare a missing target-only interface J that extends I, the byte walk cannot prove C assignable to I, and this early return emits java/lang/Object without setting hierarchyIncomplete. A later invokeinterface I can then receive an incompatible stack-map type and reject the otherwise valid hardened class; the new incomplete-hierarchy fallback is only reached below this return, so unresolved interface paths need to trigger the same discard behavior.
Useful? React with 👍 / 👎.
| for (FieldNode fn : toStrip) { | ||
| fn.value = null; | ||
| encryptedCount++; |
There was a problem hiding this comment.
When a superclass contains a precompiled or generated GETSTATIC read of a subclass's selected static final String, JVM initialization assigns the subclass's ConstantValue before initializing its superclass, so that read observes the constant. Clearing the attribute and moving the assignment into the subclass <clinit> makes the same read occur before the assignment and return null instead; unlike the jar-wide equality exclusions, this changes initialization ordering, so such fields cannot be migrated to <clinit> without accounting for superclass observations.
Useful? React with 👍 / 👎.
| try { | ||
| boolean result = e.build(distJar, r); | ||
| boolean result = e.runBuild(distJar, r); |
There was a problem hiding this comment.
When windows-source, local-windows-device, or local-linux-device uses harden.allowUnhardenedLocalBuild=true, preflight stores the decision in hardeningForceOff, but these Windows and Linux request builders never call applyHardeningRequestArgs() before runBuild. Fresh evidence beyond the process-wide-property fix is that only the Android, iOS, and JavaScript paths invoke that helper, so hardenSourceJar() sees no per-build cn1.harden.forceOff and runs hardening instead of honoring the escape hatch, producing an orphaned local mapping.
Useful? React with 👍 / 👎.
What
Adds App Hardening — a single Enterprise-gated layer that renames classes/methods/fields, encrypts string constants, and obfuscates control flow across every port (Android, iOS/ParparVM, JavaScript, native desktop) from one bytecode transform, integrated with Crash Protection so obfuscated traces are still symbolicated.
Runs on the cloud build server; this repo carries the engine and the client surface. The matching BuildDaemon PR turns it on (entitlement gate + engine invocation + mapping upload).
Pieces
maven/cn1-hardening— forked-process ProGuard+ASM engine. Demux → ProGuard rename with azq-prefixed dictionary that avoids the ParparVMNativeSymbolIndexdead-code-culler pathology → string encryption (LDC andstatic finalConstantValue, per-class decoder) → opaque-predicate control flow (safe platforms) → mangle-collision guard →CheckClassAdapterverify → mapping with provenance +mappingId. Android keeps R8 as its sole renamer.maven/cn1-retrace— ProGuard mapping parse/chain + the ParparVM trace-string parser that on-deviceThrowable.getStackTrace()now mirrors, + a local retrace CLI.rawStack/traceFormat/mappingId/hardenLevelpayload fields,PiiScrubber.scrubRawStack, cause-chain capture.harden.*hints,HardeningPreflight(fails local/source targets, invalid level, on-device-debug),Executor.hardenSourceJar/runBuild, read-onlyHardeningstatus API, simulator hint schema, newApp-Hardening.asciidoc.build_keyliteral,BuildHintEditorgrouped-Select.valueslookup,security.asciidoc"obfuscates by default" overclaim, ParparVMgetStackTrace()(fixes today's empty-frames iOS crash reports).Tests
25 unit tests green across the two modules and the crash payload: full ProGuard round-trip + behaviour preservation, string round-trip + plaintext-absence, control-flow verification, mapping retrace + chaining, trace-format detection (incl. V8/SpiderMonkey JS rejection), and the pre-flight truth table. Core, plugin, and javase compile.
Follow-ups (not blocking)
Device end-to-end retrace and the "did obfuscation break the app" matrix need a real cloud build. Android R8
-applymapping/keep export,parparvm-symbols.txtnative-address symbolication, daemon runtime stamping ofcn1.mappingId/cn1.hardened,HardeningVerifier(Check 3), and SpotBugs-gate wiring for the new modules are noted for later.Pairs with the BuildDaemon
app-hardeningPR.🤖 Generated with Claude Code