Skip to content

fix(security): send HSTS preload on every response path - #1313

Merged
IAmJSD merged 2 commits into
LeanerCloud:mainfrom
IAmJSD:fix/issue-14
Oct 25, 2026
Merged

IAmJSD merged 2 commits into
LeanerCloud:mainfrom
IAmJSD:fix/issue-14

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

What

Lambda mode's HSTS header dropped preload, while HTTP mode sends it (#14). API responses (internal/api/handler.go) and the container health endpoint (internal/server/health.go) lacked it as well.

The value now lives in one exported constant, api.HSTSHeaderValue (max-age=31536000; includeSubDomains; preload). All four response paths use it:

  • HTTP-mode static responses
  • Lambda static responses
  • API responses
  • the health endpoint

That way the modes can't drift apart again.

preload only makes the domain eligible for the browser preload list. Nothing is submitted. Submitting to hstspreload.org stays a separate manual step, and so does the decision to do it.

Closes #14

Testing

On a14dc8f (main):

  • Build and vet: go build ./... passes. go vet passes with and without -tags integration on internal/server and internal/api.
  • Tests: go test -race -count=1 ./internal/server/ ./internal/api/ passes.
    • The new TestSecurityHeaders_HSTSMatchesAcrossModes pins the Lambda, HTTP-mode and health headers to the constant and asserts it contains preload.
    • The six API assertions in handler_security_test.go now expect preload.
  • Lint and complexity: golangci-lint v2.10.1 reports 0 issues with and without the integration tag. gocyclo -over 10 is clean on the changed files.
  • git-secrets: scripts/test-git-secrets-allowlist.sh gives 137 PASS. A git-secrets scan of the changed files in an isolated repo is clean, and the planted key was flagged.

🤖 Generated with Claude Code

Lambda mode's static responses sent "max-age=31536000; includeSubDomains"
while HTTP mode sent the same value plus "; preload". API responses and
the health endpoint also lacked it. Move the value into one exported
constant, api.HSTSHeaderValue, used by all four paths, and add a test that
pins the Lambda, HTTP-mode and health headers to it.

"preload" only makes the domain eligible for the browser preload list;
submission to hstspreload.org remains a separate manual step.

Closes LeanerCloud#14

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD IAmJSD added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm impact/all-users Affects every user urgency/eventually No deadline effort/xs Trivial / one-liner type/security Security finding labels Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 6 billable files and costs up to $1.50.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 503eb2e and 6beac96.

Files selected for processing: 6
  • internal/api/handler.go
  • internal/api/handler_security_test.go
  • internal/server/health.go
  • internal/server/http.go
  • internal/server/lambda.go
  • internal/server/lambda_coverage_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Autofix could not be updated. Open Coding to check access and billing.

Brings in main's CI fixes (LeanerCloud#1236, LeanerCloud#1252) and CI changes since, so this PR gets a fresh CI run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 6beac96ecaeed36b6d979969cd06c87d8d8d097e

Blocking:

  • none

Non-blocking:

  • internal/api/handler.go:714-722: the new const and its doc block went in between // setSecurityHeaders adds comprehensive security headers to the response. and func setSecurityHeaders. As a result the const's godoc now opens with the setSecurityHeaders sentence, and setSecurityHeaders has no doc comment. Moving that one line down to sit above the func fixes it. Lint doesn't flag it (0 issues), so it's cosmetic only.
  • internal/server/health.go:217 still sends X-XSS-Protection: 1; mode=block, while the API path dropped it as deprecated. It's out of scope for sec(frontend): HSTS preload directive missing in Lambda mode — drift from HTTP-mode security headers #14 and only noted as a related drift between the response paths.

Attacked:

  • Grepped the whole tree for other HSTS sources. The Go paths are http.go, lambda.go, health.go and api/handler.go, and all four now use api.HSTSHeaderValue. The terraform frontend modules for aws, gcp and azure already send preload, so nothing is left drifting. preload without a submission doesn't change behaviour, which matches the issue's fix and acceptance.
  • Fail-before: I reverted only the lambda.go value to max-age=31536000; includeSubDomains. TestSecurityHeaders_HSTSMatchesAcrossModes then fails ("Expected ...; preload, got ...includeSubDomains"). After restoring the file, it passes.
  • Ran go build ./..., then go vet with and without -tags integration on internal/api and internal/server. Ran go test -race -short -count=1 ./internal/api/ ./internal/server/, which passes. golangci-lint v2.10.1 on both packages reports 0 issues.

Body-stale:

  • none

Reviewer: rB7. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

This was referenced Oct 25, 2026
@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 6beac96ecaeed36b6d979969cd06c87d8d8d097e merged onto main (5aa3df5b), together with #1155, #1219, #1221, #1223, #1224, #1225, #1226, #1227, #1256, #1262, #1266, #1270, #1272, #1275, #1283, #1286, #1293, #1297, #1299, #1300, #1301, #1304, #1307, #1311, #1313, #1325 in that order (combined tree f3a3331f, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 25, 2026 20:33
@IAmJSD
IAmJSD merged commit 5a6608d into LeanerCloud:main Oct 25, 2026
34 checks passed
@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on 6beac96e, green CI, and the combined-tree verification above. Squash-merged at exactly 6beac96ecaeed36b6d979969cd06c87d8d8d097e.

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

Labels

effort/xs Trivial / one-liner impact/all-users Affects every user priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/security Security finding urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sec(frontend): HSTS preload directive missing in Lambda mode — drift from HTTP-mode security headers

2 participants