Skip to content

Bump on-headers and morgan#1511

Open
dependabot[bot] wants to merge 1 commit intomasterfrom
dependabot/npm_and_yarn/multi-1083d179d6
Open

Bump on-headers and morgan#1511
dependabot[bot] wants to merge 1 commit intomasterfrom
dependabot/npm_and_yarn/multi-1083d179d6

Conversation

@dependabot
Copy link
Copy Markdown

@dependabot dependabot Bot commented on behalf of github Jul 17, 2025

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
    You can disable automated security fix PRs for this repo from the Security Alerts page.

---
updated-dependencies:
- dependency-name: on-headers
  dependency-version: 1.1.0
  dependency-type: indirect
- dependency-name: morgan
  dependency-version: 1.10.1
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot Bot added dependencies Pull requests that update a dependency file javascript Pull requests that update javascript code labels Jul 17, 2025
@MelvinTo
Copy link
Copy Markdown
Contributor

PR Review Summary

✅ What looks good

  • Reasonable targeted dependency update:
  • morgan upgraded from ^1.9.1 to ^1.10.1, which is a stable maintenance/security improvement.
  • No application logic changes in this PR, so functional regression risk from business code is low.
  • Lockfile is regenerated and consistent with dependency metadata changes.

⚠️ Issues found

  • Medium — lockfile format migration risk (lockfileVersion: 3).
    The PR upgrades npm lockfile format from v1 to v3. This can break or diverge installs on environments still using older npm (notably npm 6).
  • Low — larger-than-necessary lockfile churn for a single package bump.
    The change introduces broad structural rewrites (dependenciespackages) that make audit and rollback harder.
  • Process gap — issue requirement verification:
    No linked issue found.
    Based on provided context, there’s no issue reference to validate against acceptance criteria.

💡 Suggestions

  • Verify/standardize Node + npm versions across local dev, CI, and release environments before merging lockfile v3 changes.
  • If compatibility with older npm is still required, regenerate lockfile using the project-standard npm version first.
  • Add PR validation notes:
  • npm ci success on CI image,
  • smoke test / unit test results,
  • why this morgan update is needed (security advisory or bugfix context).

Verdict

COMMENT


Repo: firewalla/firerouter
PR: #1511
Head SHA: 05dfaa8e0c83470a07c051c2ec439aef2dad0d01
Checked at: 2026-03-19 16:34:07 CST

@j-sallyjin
Copy link
Copy Markdown

PR Review Summary

✅ What looks good

  • morgan bump from ^1.9.1 to ^1.10.1 is reasonable and brings in upstream fixes/maintenance improvements.
  • The direct runtime intent is clear and narrowly scoped in package.json.
  • Lockfile captures the expected transitive updates for the new morgan version (including on-headers/depd adjustments).

⚠️ Issues found

  • High – lockfile format migration risk (lockfileVersion: 13).
    This PR mixes a small dependency bump with a major npm lockfile format change. If any CI/device/runtime path still uses older npm, installs can fail or become non-reproducible.
  • Medium – lockfile churn is disproportionate to the functional change.
    The PR becomes much harder to audit because most diff is structural lockfile rewrite rather than meaningful dependency intent.
  • Medium – missing environment validation evidence.
    No proof here that npm ci / startup tests were executed on the actual FireRouter runtime matrix after introducing lockfile v3.
  • Issue requirement coverage: No linked issue found.

💡 Suggestions

  • Split this into two PRs:
  1. morgan bump only (minimal lockfile churn in compatible format),
  2. npm/lockfile v3 migration with explicit toolchain rollout.
  • Document required Node/npm versions and confirm all build/deploy environments support lockfile v3 before merging.
  • Attach verification artifacts for this change:
  • clean install (npm ci) on target environments,
  • service startup + logging path smoke test (to validate morgan integration unchanged).

Verdict

REQUEST_CHANGES


Repo: firewalla/firerouter
PR: #1511
Head SHA: 05dfaa8e0c83470a07c051c2ec439aef2dad0d01
Checked at: 2026-03-28 00:12:57 CST

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants