chore(security): apply the 7-day dependency cooldown to every npm install path - #33
Open
devin-ai-integration[bot] wants to merge 1 commit into
Open
chore(security): apply the 7-day dependency cooldown to every npm install path#33devin-ai-integration[bot] wants to merge 1 commit into
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Owner
|
Upstream issue filed: apache#42983 Verified against Note: #48 is a duplicate of this PR — both address the missing |
This was referenced Aug 10, 2026
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.
SUMMARY
Fixes #22 and #26.
.github/dependabot.ymlsets a 7-daycooldownon every ecosystem, but that binds Dependabot only: there was no.npmrcin the repository, so any developer or CInpm installcould resolve a version published minutes ago.docker/docker-frontend.shmade that worse by running a barenpm install, discarding the lockfile that is the repository's actual protection.Two changes:
.npmrcwithmin-release-age=7in each workspace that has its own install path —superset-frontend/,superset-frontend/cypress-base/,superset-websocket/,docs/. Per-workspace rather than repo-root because npm reads project config from the local prefix (the nearestpackage.jsondirectory), so a root.npmrcwould not be read by an install run insidesuperset-frontend/.docker/docker-frontend.shnow runsnpm ciinstead ofnpm install. Nothing at that point in the build requires a lockfile-less install: the script runs against the mounted repo checkout, which containspackage-lock.json, so no comment justifyingnpm installwas needed.On the option name:
min-release-age(days,nullby default) is the npm 11 config confirmed against the docs shipped with npm 11.13.0, the version pinned insuperset-frontend/package.jsonengines—npm install --helplists it as[--before <date>|--min-release-age <days>]. npm normalises it into abeforetimestamp, which is how the verification below shows it taking effect.docs/installs with Yarn 1.22 (packageManager), which reads.npmrcfor registry settings but has no equivalent ofmin-release-age. The file is still added there so anynpminvocation in that directory is covered, and the.npmrccomment states the limitation rather than implying protection Yarn does not give.yarn install --check-cacheindocs/was re-run with the file present and is unaffected.Out of scope, noted rather than fixed:
ignore-scriptswith an allowlist (explicitly deferred in #22); and three other directories with their own lockfiles that were not in the issue's list —superset-embedded-sdk/,superset-websocket/utils/client-ws-app/, and.github/actions/— which would need the same.npmrcfor full coverage.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Node 24.16.0 / npm 11.13.0.
The setting is read, and it actually changes resolution. In
superset-websocket/,npm config listshows the project config being applied — npm convertsmin-release-age=7into a cut-off date:(
npm config get min-release-ageprintsnull; the normalisedbeforeinnpm config listis the observable form.)A resolution test in a scratch directory, with and without the same
.npmrc, againsteslint(10.8.1 published 2026-08-07, 10.8.0 on 2026-07-24):The 3-day-old release is held back; the older one is installed.
Clean
npm cistill succeeds with the file present (rm -rf node_modulesfirst in each):superset-frontend/— added 3020 packages, 32ssuperset-frontend/cypress-base/— added 202 packages, 10ssuperset-websocket/— added 202 packages, 4sdocs/—yarn install --check-cache, done in 55s (Yarn, no lockfile-less path)min-release-agedoes not interfere withnpm ci, as expected:npm ciinstalls the exact lockfile-resolved versions rather than re-resolving.Dev frontend image, running the modified script. Built
--target superset-node --build-arg DEV_MODE=true, then ran the image with the compose dev mounts (./docker,./superset-frontend,./superset) andBUILD_SUPERSET_FRONTEND_IN_DOCKER=true, executing/app/docker/docker-frontend.shunmodified:Resolved versions match the lockfile. After that container run, every installed package under
superset-frontend/node_moduleswas compared againstpackage-lock.json: 2974 packages checked, 0 version mismatches (149 lockfile entries are optional/platform-specific and absent on disk).To reproduce:
ADDITIONAL INFORMATION
pre-commit installthenpre-commit runon the staged changes: all applicable hooks passed, no failures (most hooks skipped — no Python or TS files touched).Link to Devin session: https://app.devin.ai/sessions/199fb12f436f4a94a065fe99ecd32c9b
Requested by: @jethac