chore: remove vulnerable npm deps from dev tooling - #651
Open
jvsena42 wants to merge 3 commits into
Open
Conversation
Greptile SummaryThis PR removes the translation validator’s npm dependency and migrates the standalone APNs test helper to a smaller, updated dependency tree.
Confidence Score: 4/5The 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
|
| 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
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 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.
globdependency from the translation validator. It had a single call site with a hardcoded pattern, so a small recursive file walk replaces it. This drops theglob→minimatch→brace-expansionchain behind 7 ReDoS advisories and leavesscripts/with no npm dependency at all.npm install globunpinned from the repo root on every PR and ignored the committed lockfile entirely.node-pushnotificationswith@parse/node-apnin 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 — includingrequest, 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, andnpm auditfrom 10 vulnerabilities to 0..env.examplewith placeholders.The push tool migration is wire-compatible. The notification was compiled under the old
apn@5.1.3using the optionsnode-pushnotificationsderives internally, then deep-compared against the new output: theapsbody and headers are identical. The one difference is thatapn@8emitsapns-priority: 10explicitly whereapn@5relied on it being the APNs default. The nesting of the encrypted blob underaps.alert.payloadis 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
node index.jswith a valid.p8key and device token: notification arrives on device as before.code 2and falls back to the static alert, unchanged from before, since the test payload sends{type:'payment'}rather than the encrypted blob the extension expects.regression:test-push-server → run withDEVICE_TOKENunset: exits 1 with the missing token error.regression:test-push-server → run with a badAPN_KEY_FILEpath: exits 1 with the missing key file error.Automated Checks
node_modulespresent anywhere in the repo to confirm it runs dependency-free.apsbody match, as described above.npm auditintest-push-server: 10 vulnerabilities → 0. Lockfile: 86 → 24 entries.@parse/node-apnProvider construction verified with a throwaway EC key; the real send is covered by the manual test above.