Skip to content

Replace unmaintained aab-parser to clear protobufjs CVEs (REV-36) - #37

Merged
ashirman merged 1 commit into
mainfrom
feature/rev-36
Jul 6, 2026
Merged

Replace unmaintained aab-parser to clear protobufjs CVEs (REV-36)#37
ashirman merged 1 commit into
mainfrom
feature/rev-36

Conversation

@revopushbot

Copy link
Copy Markdown
Contributor

Problem

protobufjs was flagged by 11 npm audit advisories (worst: critical 9.8 arbitrary code execution, GHSA-xq3m-2v4x-88gg), all affecting versions <= 7.6.2. It entered the dependency tree only transitively via aab-parser@1.0.1, which hard-pins protobufjs: ^6.11.2. Every reachable 6.x version (incl. the newest, 6.11.6) is vulnerable, so npm audit fix reported fixAvailable: false.

This is distinct from REV-28 (lockfile-only, no package.json changes) — that pass couldn't touch this because the fix requires changing the dependency graph.

Why a plain overrides bump was insufficient

  • overrides is honored only for the root project, so it would not protect end-users who npm install -g @revopush/code-push-cli — they'd still get the vulnerable transitive version.
  • protobufjs 7.x made import resolution strict, which exposed that aab-parser ships a broken Resources.proto (it imports an unshipped Configuration.proto). A forced 7.x bump therefore broke AAB parsing at runtime.
  • No maintained pure-JS alternative exists — the maintained options (aab-conqueror, node-bundletool) all wrap Google's Java bundletool and would impose a new JRE requirement on every user.

Fix — vendor the small piece we actually use

  • Remove aab-parser; add maintained protobufjs@^7.6.3 (→ 7.6.5) and jszip@^3.10.1 as direct deps.
  • script/utils/aab-utils.tsparseAabManifest() reproducing aab-parser's logic (jszip → decode aapt.pb.XmlNode → extract attributes), same return shape and error text, plus clearer errors and full typing.
  • script/utils/aab-resources-descriptor.ts — the exact aapt.pb schema embedded as a compiled JSON descriptor, so it ships through plain tsc with no .proto asset and no Configuration.proto import problem.
  • command-executor.ts — swap the single call site.

Verification

  • tsc build clean; protobufjs fully removed from npm audit
  • Differential test on a real 36 MB app-release.aab: old aab-parser (protobufjs 6.11.x) vs new vendored parser (7.6.5) produce byte-identical output — both extracted metadata and the full 13,252-byte decoded XmlNode JSON
  • ✅ Synthetic Buffer + file-path + missing-manifest error paths pass
  • ✅ No Java requirement added; no new lint errors

Out of scope

Remaining audit findings are in unrelated packages: diff, mocha, parse-duration, serialize-javascript, uuid, xcode (mostly devDependencies).

Closes REV-36.

🤖 Generated with Claude Code

@revopushbot

Copy link
Copy Markdown
Contributor Author

Updated (force-pushed 28d2127) to simplify the schema handling.

Change: removed the embedded script/utils/aab-resources-descriptor.ts (the full ~24 KB auto-generated aapt.pb descriptor). Since a protobuf decoder skips fields it doesn't declare, aab-utils.ts now declares only the minimal XmlNode → XmlElement → XmlAttribute slice we actually read — 6 lines of inline proto parsed with protobuf.parse():

message XmlAttribute { string name = 2; string value = 3; }
message XmlElement  { string name = 3; repeated XmlAttribute attribute = 4; }
message XmlNode     { XmlElement element = 1; }

Field numbers are the stable public wire format from AOSP frameworks/base/tools/aapt2/Resources.proto.

Net effect: diff shrinks from ~987 to ~110 insertions; one fewer file to maintain.

Re "is there an official protobuf somewhere" — checked npm; no maintained package ships the aapt2 protos as consumable JS, so vendoring the tiny slice we read is the cleanest option.

Re-verified: tsc clean, protobufjs still absent from npm audit, and the compiled parser returns byte-identical metadata to the original aab-parser on a real 36 MB app-release.aab.

protobufjs entered the tree only via aab-parser@1.0.1, which hard-pins
protobufjs ^6.11.2. Every reachable 6.x version is affected by 11 npm audit
advisories (worst: critical 9.8 RCE), and all fixes land above the pinned
range, so npm audit fix could not resolve it.

An npm `overrides` bump was insufficient: overrides are honored only for the
root project (so end-users installing the published CLI stay vulnerable), and
protobufjs 7.x's strict import resolution exposed that aab-parser ships a
broken Resources.proto (imports an unshipped Configuration.proto), breaking AAB
parsing at runtime. No maintained pure-JS alternative exists — the maintained
options wrap Google's Java bundletool and would add a JRE requirement.

Instead, vendor the small piece actually used:
- Remove aab-parser; add maintained protobufjs ^7.6.3 and jszip ^3.10.1 as
  direct deps.
- script/utils/aab-utils.ts: parseAabManifest() reads base/manifest/
  AndroidManifest.xml (a protobuf-encoded aapt.pb.XmlNode). We only read a few
  attributes off the root <manifest> element, so it declares just the minimal
  XmlNode -> XmlElement -> XmlAttribute slice (field numbers from AOSP's
  Resources.proto); the decoder skips every other field. Same return shape and
  error text as aab-parser.
- Swap the single call site in command-executor.ts.

Verified: tsc clean; protobufjs gone from npm audit; on a real 36MB
app-release.aab the vendored parser (protobufjs 7.6.5) returns byte-identical
metadata to the original aab-parser (protobufjs 6.11.x).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ashirman
ashirman merged commit 31ba240 into main Jul 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants