build(deps-dev): bump browserslist from 4.28.2 to 4.28.8 in /web - #159
Conversation
Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.28.2 to 4.28.8. - [Release notes](https://github.com/browserslist/browserslist/releases) - [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md) - [Commits](browserslist/browserslist@4.28.2...4.28.8) --- updated-dependencies: - dependency-name: browserslist dependency-version: 4.28.8 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Review: 发现以下潜在问题1.
|
|
自动化验证回读,不是 approve / 不是 request-changes / 不携带投票,也不合并。 合并需要人工评审,我没有替任何人签。 我做了什么只点了一次
结果:15/15 required check 全部 success,包括 main 上红的那一条CI run
对照: 为什么这次绿能推到 main 上不是靠"看起来差不多"。merge commit
而 这一条红正卡在别的 PR 上main 自己是红的,所以任何从
#159 合进 main 之后,这三条会随下一次检查运行自然转绿(其中 #138 需要再点一次 update branch 让 CI 重跑)。 剩下的一步
我明确没做的事没 approve、没合并、没开 auto-merge、没改 commit 内容、没 force-push、没删分支、没关任何 issue。 |
PeterGuy326
left a comment
There was a problem hiding this comment.
code owner review: dependabot dev-dep bump clears the browserslist high advisory that reds main Web→Audit; dev-only, production audit (--omit=dev) 0 vulns; 15/15 CI green on head.
… headers (#161) ## What this changes Implements the security-header split decided for this repository on 2026-09-03: **nginx is the single authority for `X-Content-Type-Options`, `X-Frame-Options` and `Referrer-Policy`, uniformly `no-referrer`, with the proxied path de-duplicated by `proxy_hide_header`; the API keeps `Content-Security-Policy`, `X-XSS-Protection` and `Content-Disposition`.** Closes #135 (the two live proxy defects) and Closes #136 (the ownership question that defect raised). This is the in-repo redo of #144, which came from a fork and is being closed under the fork-policy decision. It is not a cherry-pick of that commit — no commit from it is imported here. ### The two defects, as measured Both were real on `main@1332bf4` and neither is visible by reading the config, which is why the test starts an nginx rather than grepping one. 1. **`/assets/` lost all three headers.** An `add_header` inside a location replaces the inherited set instead of adding to it, and that block has always had `add_header Cache-Control`. Every cached bundle shipped with no `nosniff`, no `X-Frame-Options` and no `Referrer-Policy` at all. 2. **`/v1/` sent two conflicting `Referrer-Policy` values on one response.** The proxy said `same-origin`, the API says `no-referrer`, and nginx's `add_header` appends rather than replaces, so a client received both and the effective policy depended on which one the browser kept. `X-Content-Type-Options` and `X-Frame-Options` were also duplicated (same value twice). ### Why the API keeps sending the three it no longer owns `proxy_hide_header` makes the wire value nginx's, which is what "nginx 独占" requires, without deleting `nosniff`/`DENY`/`no-referrer` from `securityHeadersMiddleware`. A `memd` reached directly — the Helm path exposes the Service, and `docs/DEPLOYMENT.md` only makes the nginx guarantee for the container — keeps its defense in depth. If the intended reading was instead that the Go middleware should stop setting them, that is a one-line change here and it should be said before merge, because the alternative silently weakens the no-proxy deployment. ## Test evidence `scripts/test_nginx_security_headers.sh` renders the shipped template with the same `envsubst` filter and variables the container entrypoint uses, runs a fake upstream that answers exactly like `securityHeadersMiddleware` does, starts nginx against the config, and reads headers off the wire across five surfaces (`/`, `/assets/`, a 404 under `/assets/`, `/v1/`, `/healthz`). | run | result | | --- | --- | | template as shipped on `main` | **11 of 28 assertions fail** — 6 missing across the two `/assets/` surfaces, `same-origin` on `/` and `/healthz`, 3 duplicated on `/v1/` | | this branch | **28 of 28 pass** | | drop the `/assets/` restatement | 3 fail (`/assets/` 200) | | drop `always` from the `/assets/` restatement | 3 fail (`/assets/` 404 only) | | drop `proxy_hide_header` | 3 fail (`/v1/` duplicates) | Each of the three fix sites is therefore individually load-bearing, and the 404 surface earns its place: it is the only one that tests `always`. The harness also refuses to report success on a partial run (the assertion count is pinned), hard-fails when a caller names an `NGINX_BIN` that is not executable, and only reports `SKIP` when no nginx was found at all — so the CI leg cannot go green by measuring nothing. Executed locally against nginx **1.27.4**, the same minor the `web/Dockerfile` pins (`nginxinc/nginx-unprivileged:1.27.4-alpine3.21`). **Measured in CI on the built image, and it did go red there first.** The first run of the new step failed exactly where this paragraph expected the risk to be: `web/Dockerfile:17` ends on `nginxinc/nginx-unprivileged`, whose own build stops at `USER 101`, so `apk add` could not write the package database -- `ERROR: Unable to lock database: Permission denied`, `exit code: 99`. Commit `1c917f96` installs the four tools in a throwaway child image that returns to uid 101 afterwards, so the harness still runs unprivileged, as the shipped container does. On that head `Deployment profiles` is green and its job log carries `all 28 security-header assertions passed`, so the leg executed the contract against the nginx that ships rather than printing `SKIP`. The harness content measured above is unchanged by that commit; it touches only the workflow step. ## Not in scope here, and worth a decision Nothing in this repository sets a `Content-Security-Policy` for the SPA itself — `default-src 'none'` is an API-only value and would break the web app if applied to it. The decided split covers the three shared headers and leaves HTML/SPA CSP unowned. This PR deliberately does not invent one. ## Links and state - `Refs` the decided split; `Closes #135` / `Closes #136` on merge. - No Go code changed, so the existing `security_headers_test.go` and `content_*` tests are unaffected. - `Web` is red on this head and identically red on `main` itself (`Audit dependencies`, `browserslist <=4.28.6`, GHSA-73wf-gq98-2v4g). That is a pre-existing repository condition rather than something this branch introduced, and the queued fix is #159; the readback is on #138. - Opening this PR is not a claim that it should be merged. Independent review is the reviewers'; I have not requested or recorded one. --------- Co-authored-by: waterbro-8 <318569545+waterbro-8@users.noreply.github.com>
Bumps browserslist from 4.28.2 to 4.28.8.
Release notes
Sourced from browserslist's releases.
Changelog
Sourced from browserslist's changelog.
Commits
f2f2e6cRelease 4.28.8 versiond0787c8Update dependenciesfcf8fa9Merge pull request #939 from Jaybhade/fix/baseline-kaios-without-downstream57ecd64fix: support "including kaios" without downstream093a0f6Update EM bannerb637868Release 4.28.7 version313f465Update dependenciesc935c5aFix regexp performanced7e9e65Rewrite structure parsing to make it always fastec4a55eFix import orderMaintainer changes
This version was pushed to npm by GitHub Actions, a new releaser for browserslist since your current version.
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 rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill 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 versionwill 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 dependencywill 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.