Warn when a new origin block silently steals the default vhost - #8
Merged
Conversation
Second thing the first live run got wrong, and this one broke a working site rather than just misreporting. nginx picks the first-parsed server block for a listen address as the default when nothing is marked `default_server`, and sites-enabled is parsed in filename order. `chovy.hacker` sorts ahead of `userdirs.conf`, so adding it made it the default vhost for 443 on a box that had never declared one. Every request with no SNI or an unmatched Host then got a self-signed certificate for a Moshpit name instead of the box's real one -- which presents as the *other*, untouched sites breaking. Detected by asking the loaded config whether anything claims default_server for 443, and saying plainly what to do if not. Not fixed automatically: the remedy is to edit somebody else's server block, and this script should not reach into unrelated config to do that unasked. Verified on the box it happened to: marking the pre-existing block restored the correct certificate for no-SNI and unmatched hosts, with chovy.hacker still answering with its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
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
1 participant
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.
The second thing the first live run of
setup-origin.shgot wrong — and unlike the reload race in #7, this one actually broke a working site.What happened
nginx picks the first-parsed server block for a listen address as the default when nothing is marked
default_server, andsites-enabledis parsed in filename order.chovy.hackersorts ahead ofuserdirs.conf, so adding it made it the default vhost for :443 on a box that had never declared one.Every request with no SNI, or with a Host matching no
server_name, then got a self-signed certificate for a Moshpit name instead of the box's real Let's Encrypt cert:The nasty part is how it presents: the name you just added works fine, and the other, untouched sites appear to break. Nothing in the script's output suggested it had touched them.
The fix
Ask the loaded config whether anything claims
default_serverfor 443, and say plainly what to do if not:Deliberately not fixed automatically. The remedy is to edit a server block belonging to some other site, and a script that adds one name should not reach into unrelated config to do it unasked.
Verified
On the box it happened to. Marking the pre-existing block restored correct behaviour:
CN=chovy.hacker✗CN=dev.profullstack.com✓dev.profullstack.comCN=dev.profullstack.comCN=dev.profullstack.com✓chovy.hackerCN=chovy.hackerCN=chovy.hacker✓https://dev.profullstack.com/~anthony/back to200.sh -nclean.🤖 Generated with Claude Code