From 10501421368110edfe9e474e1a137cb1e8df7e4f Mon Sep 17 00:00:00 2001 From: waterbro-8 <318569545+waterbro-8@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:00:04 +0800 Subject: [PATCH 1/2] fix(web): make the proxy the single authority for the shared security headers nginx already set X-Content-Type-Options, X-Frame-Options and Referrer-Policy at the server level, but two of the three ways a response leaves the container did not match that intent, and one of the values contradicted the API's own. - location /assets/ declares its own add_header, which replaces the inherited set rather than adding to it, so every cached bundle shipped with none of the three. Restated in place. - Referrer-Policy was same-origin while the API sends no-referrer, and on /v1/ both reached the client as two values of one header. The proxy value is now no-referrer and the API's copies of all three are hidden on the proxied path, so nginx decides the wire value without weakening a memd that runs without a proxy in front of it. - Content-Security-Policy, X-XSS-Protection and Content-Disposition stay the API's, because they depend on what the response is. Neither failure is visible by reading the config, so scripts/ test_nginx_security_headers.sh starts a real nginx and measures the headers: 28 assertions over five surfaces, 11 of which fail on the previous config and all of which pass on this one. Both fix sites and the always flag are each individually load-bearing against a targeted mutation. --- .github/workflows/ci.yml | 17 ++ CHANGELOG.md | 11 ++ docs/DEPLOYMENT.md | 8 + scripts/test_nginx_security_headers.sh | 248 +++++++++++++++++++++++++ scripts/verify.sh | 7 + web/nginx/default.conf.template | 18 +- 6 files changed, 308 insertions(+), 1 deletion(-) create mode 100755 scripts/test_nginx_security_headers.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b636850..4edbb29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -364,3 +364,20 @@ jobs: - name: Validate deployment configuration and production images run: make test-deploy-build + + - name: Validate web security response headers + run: | + # The header contract is measured against the nginx that actually ships, + # so it runs on the image the step above built. That image carries no + # test tooling, so the tools are added to a throwaway child image + # instead of installing them as root at run time -- the harness then + # still runs as the image's unprivileged user. + printf 'FROM mem-web:deploy-validation\nRUN apk add --no-cache bash curl python3 gettext\n' \ + >"${RUNNER_TEMP}/headers.Dockerfile" + docker build --tag mem-web-headers:validation \ + -f "${RUNNER_TEMP}/headers.Dockerfile" "${RUNNER_TEMP}" + docker run --rm \ + --volume "${GITHUB_WORKSPACE}:/src:ro" \ + --env NGINX_BIN=/usr/sbin/nginx \ + mem-web-headers:validation \ + bash /src/scripts/test_nginx_security_headers.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 583287f..5c094d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,17 @@ The project publishes 0.x prerelease versions; a stable release line is not yet - Add `nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy: no-referrer` and `Content-Security-Policy: default-src 'none'` to every API response, ordered outside the CORS handler so a preflight reply carries them too. +- Make the web proxy the single authority for `X-Content-Type-Options`, + `X-Frame-Options` and `Referrer-Policy` on every response it serves, with one + `Referrer-Policy: no-referrer` instead of the `same-origin` it shipped before, + which contradicted the API's own value and let both reach a client on the + proxied path. The API's copies are hidden at the proxy rather than removed + from the API, so a `memd` running without a proxy keeps its defense in depth. + Cached assets carried none of the three: a local `add_header` for + `Cache-Control` replaced the inherited set entirely, so the set is now + restated in that location. `scripts/test_nginx_security_headers.sh` measures + the headers off a running nginx, since neither failure mode is visible by + reading the configuration. ## [0.1.1] - 2026-08-31 diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 83acc7a..801e41d 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -120,6 +120,14 @@ Terminate HTTPS at a maintained reverse proxy or load balancer. Forward to `http://127.0.0.1:8080`, preserve the `Host` and `X-Forwarded-*` headers, and set an upload-body limit at least as large as `MEM_MAX_BODY_SIZE`. +The web container is itself a reverse proxy and is the authority for +`X-Content-Type-Options`, `X-Frame-Options` and `Referrer-Policy`; it sets them +on every response it serves and drops the copies `memd` sends so they do not +arrive twice. `Content-Security-Policy`, `X-XSS-Protection` and +`Content-Disposition` come from `memd`, because they depend on what the response +actually is. If your terminating proxy sets the first three as well, set them +there or here, not both, or a client receives two values for one header. + ### Configure and start From the repository root: diff --git a/scripts/test_nginx_security_headers.sh b/scripts/test_nginx_security_headers.sh new file mode 100755 index 0000000..35ab9b9 --- /dev/null +++ b/scripts/test_nginx_security_headers.sh @@ -0,0 +1,248 @@ +#!/usr/bin/env bash +# Security response-header contract for the web reverse proxy. +# +# nginx is the single authority for X-Content-Type-Options, X-Frame-Options and +# Referrer-Policy; the API owns Content-Security-Policy, X-XSS-Protection and +# Content-Disposition. This runs a real nginx because both failure modes are +# runtime semantics that reading the config cannot prove: add_header in a nested +# block silently voids the inherited set, and an upstream header survives the +# proxy unless it is explicitly hidden. +# +# Usage: scripts/test_nginx_security_headers.sh [path-to-nginx-binary] +set -euo pipefail + +repo_root=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd) +nginx_bin=${1:-${NGINX_BIN:-}} + +if [[ -n "$nginx_bin" && ! -x "$nginx_bin" ]]; then + # An explicitly requested binary that is not there must not become a green + # run: a caller that pins NGINX_BIN is asserting that the check executed. + printf 'ERROR: requested nginx binary is not executable: %s\n' "$nginx_bin" >&2 + exit 1 +fi +if [[ -z "$nginx_bin" ]]; then + nginx_bin=$(command -v nginx || true) +fi +if [[ -z "$nginx_bin" ]]; then + printf 'SKIP: no nginx binary found (pass one as $1 or set NGINX_BIN)\n' + exit 0 +fi +for tool in curl envsubst python3; do + command -v "$tool" >/dev/null 2>&1 || { + printf 'ERROR: %s is required to drive the proxy\n' "$tool" >&2 + exit 1 + } +done + +port=${PORT:-18080} +upstream_port=${UPSTREAM_PORT:-18081} +# 3 proxy-owned headers on five surfaces, + the 3 API-owned headers required on +# /v1/ and the 2 required absent on the other four, + the not-found status guard, +# + the /assets/ Cache-Control guard. +expected_checks=28 +work=$(mktemp -d) +upstream_pid='' +nginx_pid='' +failures=0 +checks=0 + +cleanup() { + [[ -n "$nginx_pid" ]] && kill "$nginx_pid" 2>/dev/null || true + [[ -n "$upstream_pid" ]] && kill "$upstream_pid" 2>/dev/null || true + wait 2>/dev/null || true + rm -rf "$work" +} +trap cleanup EXIT HUP INT TERM + +fail() { printf ' not ok - %s\n' "$*" >&2; failures=$((failures + 1)); checks=$((checks + 1)); } +pass() { printf ' ok - %s\n' "$*"; checks=$((checks + 1)); } + +# --- render the shipped template the way the container entrypoint does --- +prefix=$work +mkdir -p "$prefix/conf" "$prefix/logs" "$prefix/run" "$prefix/html/assets" \ + "$prefix/tmp/client" "$prefix/tmp/proxy" "$prefix/tmp/fastcgi" \ + "$prefix/tmp/uwsgi" "$prefix/tmp/scgi" +printf 'console.log(1)\n' >"$prefix/html/assets/app.js" +printf 'mem\n' >"$prefix/html/index.html" + +export MEMD_UPSTREAM="http://127.0.0.1:${upstream_port}" +export MEM_MAX_BODY_SIZE=256m +export NGINX_ENVSUBST_FILTER='^(MEMD_UPSTREAM|MEM_MAX_BODY_SIZE)$' +rendered=$work/default.conf +envsubst '$MEMD_UPSTREAM $MEM_MAX_BODY_SIZE' \ + <"$repo_root/web/nginx/default.conf.template" >"$rendered" +if grep -Eq '\$\{[A-Z_]+\}' "$rendered"; then + printf 'the template left a variable unexpanded:\n' >&2 + grep -Eo '\$\{[A-Z_]+\}' "$rendered" | sort -u >&2 + exit 1 +fi + +mime_candidates=( + "$(cd -- "$(dirname -- "$nginx_bin")/.." && pwd)/conf/mime.types" + /etc/nginx/mime.types +) +mime_types='' +for candidate in "${mime_candidates[@]}"; do + if [[ -f "$candidate" ]]; then mime_types=$candidate; break; fi +done +if [[ -z "$mime_types" ]]; then + printf 'no mime.types found near %s\n' "$nginx_bin" >&2 + exit 1 +fi + +python3 - "$rendered" "$work/conf/nginx.conf" "$prefix" "$port" "$mime_types" <<'PY' +import sys + +conf_path, out_path, prefix, port, mime_types = sys.argv[1:6] +with open(conf_path, encoding="utf-8") as handle: + server = handle.read() +server = server.replace("listen 8080;", "listen 127.0.0.1:%s;" % port) +server = server.replace("root /usr/share/nginx/html;", "root %s/html;" % prefix) +if "listen 127.0.0.1:%s;" % port not in server or ("%s/html" % prefix) not in server: + raise SystemExit("the shipped server block drifted: could not rebind it for the harness") + +with open(out_path, "w", encoding="utf-8") as handle: + handle.write(""" +worker_processes 1; +error_log {prefix}/logs/error.log warn; +pid {prefix}/run/nginx.pid; +daemon off; + +events {{ worker_connections 64; }} + +http {{ + include {mime_types}; + default_type application/octet-stream; + access_log {prefix}/logs/access.log; + client_body_temp_path {prefix}/tmp/client; + proxy_temp_path {prefix}/tmp/proxy; + fastcgi_temp_path {prefix}/tmp/fastcgi; + uwsgi_temp_path {prefix}/tmp/uwsgi; + scgi_temp_path {prefix}/tmp/scgi; + +{server} +}} +""".format(prefix=prefix, server=server.rstrip(), mime_types=mime_types)) +PY + +# --- fake upstream answering exactly like the Go API middleware does --- +cat >"$work/upstream.py" <<'PY' +import http.server +import os + +# Mirrors securityHeadersMiddleware (server/internal/api/util.go) plus the +# per-response Content-Disposition that the download handlers set. If the API's +# header set changes, this fixture must change with it. +class Handler(http.server.BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def do_GET(self): # noqa: N802 + body = b'{"ok":true}' + self.send_response(200) + self.send_header("X-Content-Type-Options", "nosniff") + self.send_header("X-Frame-Options", "DENY") + self.send_header("Referrer-Policy", "no-referrer") + self.send_header("Content-Security-Policy", "default-src 'none'") + self.send_header("X-XSS-Protection", "0") + self.send_header("Content-Disposition", 'attachment; filename="note.txt"') + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + pass + +http.server.HTTPServer(("127.0.0.1", int(os.environ["UPSTREAM_PORT"])), Handler).serve_forever() +PY +UPSTREAM_PORT=$upstream_port python3 "$work/upstream.py" & +upstream_pid=$! + +"$nginx_bin" -p "$prefix" -c "$work/conf/nginx.conf" & +nginx_pid=$! + +ready='' +for _ in $(seq 1 50); do + if curl -fsS -o /dev/null "http://127.0.0.1:${port}/v1/ping" 2>/dev/null; then ready=yes; break; fi + sleep 0.2 +done +if [[ -z "$ready" ]]; then + printf 'nginx never became ready; error log:\n' >&2 + cat "$prefix/logs/error.log" >&2 || true + exit 1 +fi + +header_values() { + # No -f: a 404 or a 502 has to be probeable too, because `always` is what + # keeps these headers on an error response. + curl -sS -D - -o /dev/null "$1" | tr -d '\r' | + awk -v h="$2" ' + /^$/ { exit } + tolower($0) ~ "^" tolower(h) ":" { sub(/^[^:]*:[ \t]?/, ""); print }' +} + +check_single() { + local url=$1 header=$2 want=$3 got count + got=$(header_values "$url" "$header") + count=$(printf '%s' "$got" | grep -c . || true) + if [[ $count -ne 1 ]]; then + fail "${url##*/}: $header appears $count times, want exactly 1 ($(printf '%s' "$got" | tr '\n' '|'))" + elif [[ $got != "$want" ]]; then + fail "${url##*/}: $header = $got, want $want" + else + pass "${url##*/}: $header: $got" + fi +} + +check_absent() { + local url=$1 header=$2 count + count=$(header_values "$url" "$header" | grep -c . || true) + if [[ $count -ne 0 ]]; then + fail "${url##*/}: $header present, want absent (nginx must not set what the API owns)" + else + pass "${url##*/}: $header absent" + fi +} + +for path in /index.html /assets/app.js /assets/does-not-exist.js /v1/ping /healthz; do + printf '\n== %s ==\n' "$path" + url="http://127.0.0.1:${port}${path}" + check_single "$url" X-Content-Type-Options nosniff + check_single "$url" X-Frame-Options DENY + check_single "$url" Referrer-Policy no-referrer + if [[ $path == /v1/* ]]; then + check_single "$url" Content-Security-Policy "default-src 'none'" + check_single "$url" X-XSS-Protection 0 + check_single "$url" Content-Disposition 'attachment; filename="note.txt"' + else + check_absent "$url" Content-Security-Policy + check_absent "$url" Content-Disposition + fi +done + +printf '\n== the not-found surface really was a not-found ==\n' +status=$(curl -sS -o /dev/null -w '%{http_code}' "http://127.0.0.1:${port}/assets/does-not-exist.js") +if [[ $status == 404 ]]; then + pass "/assets/does-not-exist.js: HTTP $status" +else + fail "/assets/does-not-exist.js: HTTP $status, want 404 -- the header assertions above would then be measuring a success response, not the always flag" +fi + +printf '\n== /assets/ caching is not collateral damage ==\n' +got=$(header_values "http://127.0.0.1:${port}/assets/app.js" Cache-Control) +if [[ $got == *immutable* ]]; then + pass "/assets/app.js: Cache-Control: $got" +else + fail "/assets/app.js: Cache-Control = ${got:-}, want it to still say immutable" +fi + +printf '\n' +if [[ $checks -ne $expected_checks ]]; then + printf '%d security-header assertions ran, expected %d\n' "$checks" "$expected_checks" >&2 + exit 1 +fi +if [[ $failures -ne 0 ]]; then + printf '%d of %d security-header assertions failed\n' "$failures" "$checks" >&2 + exit 1 +fi +printf 'all %d security-header assertions passed\n' "$checks" diff --git a/scripts/verify.sh b/scripts/verify.sh index e1aa3c8..eed61b0 100755 --- a/scripts/verify.sh +++ b/scripts/verify.sh @@ -117,6 +117,11 @@ run_web() { (cd "${REPO_ROOT}/web" && npm run test:transfer) } +run_web_headers() { + log "Web reverse-proxy security headers" + "${REPO_ROOT}/scripts/test_nginx_security_headers.sh" +} + validate_test_database() { [[ -n "${MEM_TEST_DB:-}" ]] \ || die "MEM_TEST_DB is required; run: make test-env-up" @@ -423,6 +428,7 @@ case "$MODE" in run_server run_worker run_web + run_web_headers ;; race) run_race ;; integration) run_integration ;; @@ -431,6 +437,7 @@ case "$MODE" in run_server run_worker run_web + run_web_headers run_race run_integration run_integration_race diff --git a/web/nginx/default.conf.template b/web/nginx/default.conf.template index 48b4382..72de1db 100644 --- a/web/nginx/default.conf.template +++ b/web/nginx/default.conf.template @@ -7,9 +7,16 @@ server { index index.html; client_max_body_size ${MEM_MAX_BODY_SIZE}; + # nginx is the single authority for these three headers on every response + # that leaves this container, so the values below are the values a client + # sees. The API sets the same three itself for deployments that run memd + # without a proxy in front; /v1/ hides those copies so they cannot arrive + # alongside these ones. Content-Security-Policy, X-XSS-Protection and + # Content-Disposition stay the API's, because they depend on what the + # response actually is and nginx cannot know that. add_header X-Content-Type-Options "nosniff" always; - add_header Referrer-Policy "same-origin" always; add_header X-Frame-Options "DENY" always; + add_header Referrer-Policy "no-referrer" always; location = /healthz { access_log off; @@ -18,6 +25,9 @@ server { } location /v1/ { + proxy_hide_header X-Content-Type-Options; + proxy_hide_header X-Frame-Options; + proxy_hide_header Referrer-Policy; proxy_http_version 1.1; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; @@ -32,6 +42,12 @@ server { location /assets/ { try_files $uri =404; expires 1y; + # An add_header in this block replaces the inherited set rather than + # adding to it, so the three headers above have to be restated here or + # every cached bundle ships without them. + add_header X-Content-Type-Options "nosniff" always; + add_header X-Frame-Options "DENY" always; + add_header Referrer-Policy "no-referrer" always; add_header Cache-Control "public, immutable"; } From 1c917f96b89f5fa7b49b0df5100a6193c5e832b5 Mon Sep 17 00:00:00 2001 From: waterbro-8 <318569545+waterbro-8@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:00:04 +0800 Subject: [PATCH 2/2] fix(ci): install the header harness tools as root, then drop to uid 101 The `Deployment profiles` leg failed on the first run of the new step: ``` #5 0.117 ERROR: Unable to lock database: Permission denied ERROR: process "/bin/sh -c apk add --no-cache bash curl python3 gettext" did not complete successfully: exit code: 99 ``` `web/Dockerfile:17` ends on `nginxinc/nginx-unprivileged:1.27.4-alpine3.21`, whose own build stops at `USER 101`. That uid cannot write the apk database, so installing the harness tools in the image under test is not possible -- and running the step as root would measure the headers as a user the shipped container never uses. The tools therefore go into a throwaway child image: root installs, then the image returns to uid 101. The step above still builds the production image untouched, and the harness runs unprivileged against the nginx that ships in it. `NGINX_BIN` is pinned, so a missing binary fails the leg instead of printing `SKIP`. --- .github/workflows/ci.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4edbb29..a6f912f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -368,11 +368,12 @@ jobs: - name: Validate web security response headers run: | # The header contract is measured against the nginx that actually ships, - # so it runs on the image the step above built. That image carries no - # test tooling, so the tools are added to a throwaway child image - # instead of installing them as root at run time -- the harness then - # still runs as the image's unprivileged user. - printf 'FROM mem-web:deploy-validation\nRUN apk add --no-cache bash curl python3 gettext\n' \ + # so it runs on the image the step above built. That image ends its own + # build at USER 101, which cannot install packages, so the tools go into + # a throwaway child image that returns to that user id afterwards -- the + # harness then still runs unprivileged, as the container does in + # production. + printf 'FROM mem-web:deploy-validation\nUSER root\nRUN apk add --no-cache bash curl python3 gettext\nUSER 101\n' \ >"${RUNNER_TEMP}/headers.Dockerfile" docker build --tag mem-web-headers:validation \ -f "${RUNNER_TEMP}/headers.Dockerfile" "${RUNNER_TEMP}"