Skip to content

chore: remove vulnerable npm deps from dev tooling - #651

Open
jvsena42 wants to merge 3 commits into
masterfrom
chore/dependabot-npm-cleanup
Open

chore: remove vulnerable npm deps from dev tooling#651
jvsena42 wants to merge 3 commits into
masterfrom
chore/dependabot-npm-cleanup

Conversation

@jvsena42

@jvsena42 jvsena42 commented Aug 4, 2026

Copy link
Copy Markdown
Member

Description

This PR clears all 22 open Dependabot alerts. Every one of them sits in the repo's two npm helper directories, neither of which is part of the app build — the only script phase in the Xcode project strips static framework stubs, and there is no node or npm reference in the project file. Rather than dismissing them again, the vulnerable packages are removed or upgraded outright.

  1. Removes the glob dependency from the translation validator. It had a single call site with a hardcoded pattern, so a small recursive file walk replaces it. This drops the globminimatchbrace-expansion chain behind 7 ReDoS advisories and leaves scripts/ with no npm dependency at all.
  2. Drops the install step from the translation workflow, which ran npm install glob unpinned from the repo root on every PR and ignored the committed lockfile entirely.
  3. Replaces node-pushnotifications with @parse/node-apn in the push test tool. The wrapper pulled in the Android, WebPush, Windows and Amazon transports that this Apple-only tool never calls, and those dead branches carried 9 advisories — including request, which is deprecated with no patched version available. The remaining 6 advisories are fixed by the upgraded Apple branch. The dependency tree goes from 86 entries to 24, and npm audit from 10 vulnerabilities to 0.
  4. Replaces the real Apple key and team IDs in .env.example with placeholders.

The push tool migration is wire-compatible. The notification was compiled under the old apn@5.1.3 using the options node-pushnotifications derives internally, then deep-compared against the new output: the aps body and headers are identical. The one difference is that apn@8 emits apns-priority: 10 explicitly where apn@5 relied on it being the APNs default. The nesting of the encrypted blob under aps.alert.payload is deliberate and preserved, since that is where the notification service extension reads it from, and the wrapper's implicit 28 day expiry is carried over so the expiration header does not silently change.

No app code is touched, so there is no changelog fragment.

Linked Issues/Tasks

N/A

Screenshot / Video

N/A

QA Notes

Manual Tests

  • 1. test-push-server → run node index.js with a valid .p8 key and device token: notification arrives on device as before.
    • 1b. Device logs → inspect the received payload: decryption still fails at code 2 and falls back to the static alert, unchanged from before, since the test payload sends {type:'payment'} rather than the encrypted blob the extension expects.
  • 2. regression: test-push-server → run with DEVICE_TOKEN unset: exits 1 with the missing token error.
  • 3. regression: test-push-server → run with a bad APN_KEY_FILE path: exits 1 with the missing key file error.

Automated Checks

  • Translation validator output is byte-identical before and after the change (2024 lines, exit 0), verified with no node_modules present anywhere in the repo to confirm it runs dependency-free.
  • APNs wire payload deep-compared across the migration; headers and aps body match, as described above.
  • npm audit in test-push-server: 10 vulnerabilities → 0. Lockfile: 86 → 24 entries.
  • @parse/node-apn Provider construction verified with a throwaway EC key; the real send is covered by the manual test above.
  • CI: standard checks run by the PR bot. The Validate Translations workflow now runs with no install step.

@jvsena42 jvsena42 self-assigned this Aug 4, 2026
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes the translation validator’s npm dependency and migrates the standalone APNs test helper to a smaller, updated dependency tree.

  • Replaces glob-based Swift discovery with a dependency-free recursive filesystem walk and removes the workflow install step.
  • Migrates push construction and delivery from node-pushnotifications to @parse/node-apn.
  • Updates the push helper lockfile and replaces credential-like example values with placeholders.

Confidence Score: 4/5

The PR appears safe to merge, with the non-blocking caveat that the push helper should advertise its newly required Node runtime.

The translation and APNs migrations preserve the established behavior, while the only accepted concern is missing runtime documentation and package metadata for @parse/node-apn’s Node 20/22/24 requirement.

Files Needing Attention: test-push-server/package.json and test-push-server/README.md

Important Files Changed

Filename Overview
scripts/validate-translations.js Replaces the single glob call with deterministic recursive Swift-file collection; no current behavioral defect was established.
.github/workflows/validate-translations.yml Removes the unnecessary runtime installation now that the validator uses only Node built-ins.
test-push-server/helpers.js Constructs the APNs notification directly while preserving the extension-specific alert payload and prior expiration behavior.
test-push-server/index.js Migrates sending, result handling, and provider shutdown to the direct APNs API without an established changed-code failure.
test-push-server/settings.js Converts wrapper configuration into direct Provider options and uses the numeric APNs priority expected by the new library.
test-push-server/package.json Replaces the broad push wrapper with @parse/node-apn but does not declare its Node 20/22/24 runtime requirement.
test-push-server/package-lock.json Shrinks and upgrades the dependency tree while introducing a direct dependency with a narrower Node engine range.

Reviews (1): Last reviewed commit: "chore: replace node-pushnotifications wi..." | Re-trigger Greptile

Comment thread test-push-server/package.json
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.

1 participant