Reference in New Issue
Block a user
Delete Branch "issue-14-security-headers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #14.
One commit, two files: a new
static/_headersandTODO.md.The one thing a reviewer must not skip
These headers are unverified in production and cannot be verified from this
branch. Cloudflare Pages silently ignores a malformed
_headersfile, so thegreen build and the browser testing below prove the policy is correct and
prove nothing about whether Pages parses the file. That check needs a live
deploy and belongs after merge.
When it is done, it must be made on
strict-transport-securityorcontent-security-policy. Checkingx-content-type-optionswould be a falsepass: Cloudflare already sends that one whether or not the file took effect. It
must be run against both hostnames, not just the apex — dropping
includeSubDomainsrests onwww.lora.vegasbeing served by this same Pagesproject, and that was established behaviourally rather than from the Cloudflare
dashboard:
If
wwwturns out not to be covered, theincludeSubDomainsdecision has to berevisited.
Before state, measured
curl -sSI https://lora.vegas/, run against the live site while writing this:No
strict-transport-security, nocontent-security-policy, nox-frame-options, nopermissions-policy. This confirms the correction in theissue comment rather than the issue body's "no response security headers"
opening: two of the six are already served by Cloudflare's defaults, so those
two lines in the file are restatements and the substance of this change is the
other four.
Also confirmed:
https://lora.vegas/andhttps://www.lora.vegas/returnbyte-identical bodies, and both hash equal to the
public/index.htmlthisbranch builds. Same Pages project, two custom domains, deployment current with
main. That fact decides the HSTS question below.The headers
The file carries its reasoning in
#comments, which Cloudflare's_headersformat supports.
CSP
Checked against the built page, not copied from a template.
public/index.htmlcontains no
script,img,link,iframe,form,video,audio,objectorembedelement, nostyle=attribute and noon*=handler. It hasexactly one inline
<style>block, whichthemes/loravega/layouts/_default/baseof.htmlfills with the whole ofthemes/loravega/static/css/style.cssviareadFile.So
default-src 'none'holds, and'unsafe-inline'instyle-srcis requiredby — and only by — that inlining, which is a deliberate theme design choice. No
script-srcallowance, because there are no scripts to allow.img-src 'self'is kept although the page has no images. Measured both ways inChrome: with the allowance the browser's unprompted
/favicon.icorequest ismade and 404s; without it the request is suppressed entirely. Neither is a
problem today, so this is a forward-looking same-origin allowance, not a fix —
it loosens nothing cross-origin. (My plan comment on the issue claimed the
bare-
default-srccase logs a violation event; that turned out to be wrong, andthe file comment records what actually happens.)
X-Frame-Options: DENYandframe-ancestors 'none'agree. Nothing in the repoor the issue suggests an intent to let others embed the channel listings; if
that changes, both move together.
HSTS: no
includeSubDomains, nopreloadpreloadis omitted for the reason the issue gives — effectively irreversible,an owner decision.
includeSubDomainsis dropped, and the deciding evidence is thatwww.lora.vegasis served by this same Pages project, so this_headersblocksets HSTS on its responses directly. That is the concrete present-day gap
includeSubDomainswould otherwise close, and it is closed without it. Beyondwww, no other name underlora.vegasresolves at all (checkedmqtt,map,meshmap,mail,api,dev,mesh,node,admin,test,staging).So
includeSubDomainsbuys nothing today, while committing every futuresubdomain to HTTPS-only in browsers for a year. It is the half-step to
preload, and it sharespreload's asymmetry: it cannot be walked back insidethe max-age window without also dropping the apex protection. For a community
mesh site where a
map.ormqtt.host on someone's home server is a plausiblenext move, that is the owner's call to make deliberately, not mine to make by
copying a baseline. Left as a tracked decision in
TODO.md.access-control-allow-origin: *Left alone, deliberately. It is Cloudflare's default for static assets and this
site has no credentialed endpoints, no cookies, no API and no private origin
behind it — every byte it serves is already public, so permitting cross-origin
reads grants nothing that a plain fetch of the public URL does not. Overriding
it to a narrower value would be pure noise with a real downside: it would break
any third party legitimately fetching the page. Recorded as a decision rather
than left unexamined, per the issue comment.
Permissions-Policy
The issue's three, unchanged. The site uses no browser features at all, so a
longer list would add unrecognized-feature console warnings without adding
protection.
Verification
Deploy path unaffected. This touches no script, no
Makefiletarget, noDockerfileand neither.gitea/workflows/deploy.ymlnorscript/bootstrap.It adds one static file to the build output.
The theme's
static/is not shadowed. This is the first root-levelstatic/in the repo, and that was the real risk. Hugo unions the project'sstatic/over the theme's per path, so a rootstatic/_headerscannot displacethemes/loravega/static/css/style.css. Proved rather than asserted, by buildingbefore and after:
public/css/style.cssis unchanged atd5b8acd561ca7792de2c81f62e773d743074840fdd547cc642692c090302366a,public/index.htmlunchanged atca6ff211353833806c1f36b998ea3b436c573f39c92293d0dcd7b135d852d866, the treegains exactly one file, and Hugo's static file count goes 1 to 2.
The file lands where Pages reads it. After
make test,public/_headersexists and is byte-identical to
static/_headers.The CSP does not break the page. Not reasoned about — loaded. I served the
built
public/from a local HTTP server that parses the committed_headersand emits it as real response headers (real headers matter:
frame-ancestorsand
X-Frame-Optionsdo not work from a<meta>tag), then drove it withheadless Chrome 137 while collecting
securitypolicyviolationevents, consoleerrors and failed requests:
computed style is the theme's, not the browser's: body padding
32px 16px,max-width
90%, line-height25.6px, taglinergb(102, 102, 102), linksrgb(0, 102, 204)— the--mutedand--accentvalues fromstyle.css.yields "Refused to frame ... because an ancestor violates ... frame-ancestors
'none'" and an empty frame.
CSP-protected document: meshtastic.org, signal.group, discord.gg (redirects to
discord.com/invite/...), sneak.berlin, git.eeqj.de — every one status 200,
zero CSP violations on navigation. As expected: no directive in this policy
governs top-level anchor navigation.
/favicon.ico404 discussed above, and onesgnl://ERR_ABORTEDfrom signal.group's own protocol-handler attempt afternavigation, which is that site's behaviour and not ours.
make checkpasses: cleanhugo --minifybuild,--printPathWarningslintbuild, prettier check.
make fmtleaves the tree unchanged._headershas noextension and so is outside prettier's
**/*.md/**/*.cssscope.script/cibuildsucceeds with the check layer genuinely executed, notCACHED— on the reworked head the layer ran 2.4s and emitted both Hugo buildsand the prettier output, with
Static files 2confirming the new file is in theimage build too.
TODO.md
The Workflow section's rotation was not applied, deliberately: Next Step is
LICENSE(#10), which is neitherthis work nor done — it is blocked on the owner's choice of license. Moving it
to Completed Steps would be a false claim. Instead this adds the Completed entry
for this work, updates Status, and adds two Future Steps: the post-merge live
header check on both hostnames, and the deferred HSTS
includeSubDomains/preloaddecision.Review: FAIL —
needs-reworkstatic/_headersis correct and needs no change. Both blocking defects are inTODO.md; the fix is an edit plus an amend.Blocking
1.
TODO.md, Completed Steps bullet (last line of the new entry) — sentencetruncated mid-clause. The bullet ends:
No terminal punctuation, and the clause that carries the actual point is gone.
The same sentence in the commit message reads "... inside the max-age window
without also dropping the apex protection." The
TODO.mdcopy lost it. Thisis the repo's canonical planning document and it now contains an incomplete
sentence. Acceptable: restore the dropped clause and the full stop.
2.
TODO.md, Completed Steps bullet — HTML entities inside a Markdown codespan. The entry contains
`<style>`. CommonMark does notdecode entity references inside code spans, so Gitea renders that literally as
<style>rather than as the tag. The escaping is required for trackercomments, not for a file in the repo — no other Markdown in this repo uses
entities. Acceptable:
`<style>`.Non-blocking
3. Commit subject is 80 characters. Every other subject on
mainis 48-67.An amend is needed for the above anyway; shorten to <= 72.
4. The post-merge verification step names only the apex.
www.lora.vegasiscovered by this file only if it is the same Pages project. That was established
behaviorally, not from the Cloudflare dashboard — and since dropping
includeSubDomainsrests entirely on that inference, the check that proves itshould be part of the follow-up. Suggest the Future Steps entry and the issue
report also curl
https://www.lora.vegas/. Ifwwwturns out not to be covered,the
includeSubDomainsdecision has to be revisited.Verified independently, passing
public/index.htmlhas noscript,img,link,iframe,form,baseor media element, nostyle=and noon*=; exactly one inline<style>. The inlined CSS has zerourl(, zero@import, zero@font-faceand exactly 17 rule blocks — independentlymatching the reported "17 CSS rules". All 12 anchors are top-level navigations,
governed by no directive in this policy. So
default-src 'none'+style-src 'unsafe-inline'is sufficient and nothing else is needed.frame-ancestors 'none'andX-Frame-Options: DENYagree.main, hashed, built the head, hashed:public/css/style.cssd5b8acd5...andpublic/index.htmlca6ff211...byte-identical across both, static count 1 to 2,
public/_headerspresentafter
make testand byte-identical tostatic/_headers.curl -sSIon apex andwwwreproduces the PR body exactly: no
strict-transport-security,content-security-policy,x-frame-optionsorpermissions-policy;x-content-type-optionsandreferrer-policypresent;access-control-allow-origin: *.Both bodies hash to
ca6ff211..., the same as the local build. Re-probed DNS:of
www mqtt map meshmap mail api dev mesh node admin test staging ftp vpn home,only
wwwresolves. The reasoning holds subject to finding 4.img-srccomment is now truthful — it records measured behaviour and nolonger claims a violation event. See disclosure below.
Permissions-Policyis right-sized for a sitethat uses no browser features. Nothing meaningful is missing: COOP/CORP buy
nothing on a scriptless page, and
X-XSS-Protectionis correctly absent.Correctly did not add
upgrade-insecure-requests, which would have beenpointless here and is adjacent to the one plaintext outbound link
(
http://las.packetflood.net:44920/, a cross-origin top-level navigation andtherefore unaffected either way).
make checkgreen and tree unmodified;script/cibuildgreen with the checklayer genuinely executed (layer #11, 2.3s, both Hugo builds and prettier output
emitted,
Static files 2) — notCACHED; CIsuccesson8d639aa;mergeable, merge-base is current
main821a293; single commit; two filesonly, no debris; deploy path untouched (
tarpicks uppublic/_headers,wrangler pages deploy publicconsumes it); title ends(closes #14);_headersis outside prettier's scope; no prohibited attribution anywhere.Disclosures
test. I verified statically instead, as above. With zero subresources on the
page that audit covers the same ground, with one exception: the favicon /
img-srcbehaviour is reported as measured and I could not re-measure it.Nothing rests on it either way.
states this plainly and names
strict-transport-security/content-security-policyas the headers to check, calling outx-content-type-optionsas a false pass. Both required statements present.TODO.mdWorkflow rotation was deliberately not applied(Next Step is #10, blocked on the owner). Consistent with how
#35 was handled. Not a finding.
FAIL accepted. Relabelled
needs-rework; dispatching rework, then a freshreviewer.
static/_headersitself is correct and is not being touched.Finding 2 is my fault, not the implementer's. I instruct every agent to
escape
<and>as HTML entities in Gitea bodies, because the trackermangles them otherwise. I never scoped that to tracker text, so it was applied
to a repo file —
TODO.mdnow contains`&lt;style&gt;`inside a Markdowncode span, which CommonMark does not decode, so Gitea renders the entities
literally. Tracker-comment escaping leaked into version control because my
instruction did not say where it stops. Every dispatch from here says the rule
applies to Gitea comment and PR bodies only and never to files in the repo.
Finding 1 stands on its own: the
TODO.mdsentence is truncated mid-clause andloses the actual reasoning. The commit message has the complete version
("...without also dropping the apex protection"), so the two disagree — and the
one that survives in the repo is the incomplete one.
Findings 3 and 4 folded into the same rework since an amend is required anyway.
Finding 4 is the substantive one: dropping
includeSubDomainsrests entirelyon
www.lora.vegasbeing the same Pages project, established behaviourallyrather than from the dashboard, so the post-merge check must cover
wwwandnot just the apex.
Worth recording about the review itself: no headless browser exists on that
host, so it could not re-run the author's browser test and said so rather
than implying it had. It verified statically instead — zero
script/img/link/iframe/form/baseelements, no inline handlers, one<style>block, zero
url(/@import/@font-facein the CSS, and independently counted17 rule blocks matching the author's figure. With zero subresources that covers
the same ground, except the favicon behaviour, which nothing rests on. A
reviewer naming the one thing it could not reproduce is worth more than one
that quietly reruns a weaker check.
Not filed, for @sneak:
style-src 'unsafe-inline'could be a'sha256-...'source expression instead, which would satisfy the policy's preference against
unsafe-inlinemore strictly. Cost is real — it couplesstatic/_headerstostyle.cssbytes, so any CSS edit silently breaks styling until the hash isupdated, and it needs
_headersmoved out ofstatic/into a Hugo outputformat. #14 explicitly blesses
unsafe-inlinehere given the theme inlines CSSby design. Say if you want it as a follow-up.
8d639aa258to5f998c6e70Rework pushed:
8d639aaamended to5f998c6Still one commit.
static/_headersis untouched —git diff 8d639aa HEAD -- static/_headersis empty. The entire diff against thereviewed head is
TODO.md, 12 insertions and 6 deletions.1. Truncated sentence — fixed
The Completed Steps bullet now ends:
> ... and
includeSubDomainswould instead bind every future subdomain for a> year with no way to walk it back inside the max-age window without also
> dropping the apex protection.
Restored clause and terminal full stop, so
TODO.mdnow carries the samereasoning as the commit message instead of half of it.
2. HTML entities in a repo file — fixed
`&lt;style&gt;`in the Completed Steps bullet is now`<style>`with literal characters. Confirmed it was the only leak: grepped the whole
worktree's Markdown for
&lt;,&gt;,&amp;,&quot;and numeric characterreferences — that line was the sole hit, and there are now zero.
3. Commit subject shortened — done
Was 80 characters. Now 71, within the 48-67 neighbourhood of the rest of
mainand under the 72 limit:
Still ends with
(closes #14). The body is otherwise unchanged except for thelast paragraph, see below.
4. Post-merge check now covers
www— doneThe Future Steps entry names both hostnames and records why
wwwis notoptional: dropping
includeSubDomainsrests entirely onwww.lora.vegasbeingthis same Pages project, which was established from identical response bodies
and not from the Cloudflare dashboard. It now reads, in part:
> After the next deploy, confirm the
_headersfile actually took effect, on> both
https://lora.vegas/andhttps://www.lora.vegas/:curl -sSIagainst> each must show
strict-transport-securityorcontent-security-policy. ...>
wwwhas to be checked too and not just the apex: dropping>
includeSubDomainsrests onwww.lora.vegasbeing served by this same Pages> project, which was established behaviourally from identical response bodies
> rather than from the Cloudflare dashboard. If
wwwturns out not to be> covered, the
includeSubDomainsdecision has to be revisited (#14)Carried through to the two other places that instruct the same check, so they do
not disagree with each other: the commit message's closing paragraph and the PR
body's curl snippet, which now runs against both hostnames.
Verification on
5f998c6make checkgreen:hugo --minifybuild clean,--printPathWarningslintbuild clean,
Static files 2, prettierAll matched files use Prettier code style!.make fmtleaves the tree unmodified —TODO.mdreportedunchanged, so theedits were already conforming under the widened
'**/*.md'scope, andmake fmt-checkis consequently clean.script/cibuildgreen with the check layer genuinely executed, notCACHED: layer#11 [7/7] RUN echo "check epoch: ..." && make checkranDONE 2.4sand emitted both Hugo builds and the prettier line, withStatic files 2.RUN script/bootstrapstayedCACHED, as intended.git diff 8d639aa HEAD -- static/_headersempty;git statusclean; singlecommit on the branch; merge base still current
main821a293391.Force-pushed with
--force-with-leasepinned to8d639aa, so the push wouldhave refused had the branch moved.
Not re-verified in this pass, and unchanged from the reviewed head since the
file it concerns was not touched: the headless-Chrome CSP run and the live
curlprobes. Label leftneeds-reworkand assignment left onclawbotforthe relabel.
Re-review of
5f998c6: PASSFresh reviewer, did not author the change and did not write the earlier review.
Scoped as instructed: the prior review passed
static/_headerson the merits, sothis pass targets the rework delta and the four findings, not a re-derivation of
the CSP.
The critical check — nothing beyond the reported defects moved
git diff 8d639aa 5f998c6touchesTODO.mdonly, 12 insertions / 6deletions.
git diff 8d639aa 5f998c6 -- static/_headersis empty, and--statover all ofstatic/is empty. The file the prior review cleared on themerits is byte-identical at the candidate head, so that analysis carries forward
by construction rather than by assertion.
The four findings, each verified closed
...inside the max-age window without also dropping the apex protection.with the terminal period, carrying the same reasoning as the commit message.
`<style>`with literal characters. I ran the sweep myself rather than taking the report:
git grep -nE '&(lt|gt|amp|quot|apos|nbsp|#[0-9]+|#x[0-9A-Fa-f]+);'over allfive tracked Markdown files (
README.md,REPO_POLICIES.md,TODO.md,archetypes/default.md,content/_index.md) returns zero hits. Widened tothe entire tree, the sole hit is
themes/loravega/layouts/index.html:3Meshtastic & LoRa— a correct entity in an HTML file, pre-existing,outside this diff.
(closes #14).wwwin the post-merge step — all three locations agree, checkedindependently rather than trusting the report:
TODO.mdFuture Steps namesboth
https://lora.vegas/andhttps://www.lora.vegas/; the commit message'sclosing paragraph says "against both lora.vegas and www.lora.vegas"; the PR
body's curl snippet runs both. The superseded apex-only entry was replaced, not
duplicated —
grepfinds exactly one instruction line inTODO.md.Checks
make fmt-checkclean andmake checkgreen with the tree unmodified after(
Static files 2).script/cibuildgreen in 8.5s with the check layergenuinely executed — layer
#12 [7/7] RUN echo "check epoch: ..." && make checkranDONE 4.1sand emitted both Hugo builds and the prettier line;RUN script/bootstrapstayedCACHED, as theCHECK_EPOCHdesign intends. CIsuccesson5f998c6.public/_headerspresent after the build andbyte-identical to
static/_headers.Probed rather than assumed: the prettier pass on
TODO.mdcould have been anignore-file artifact, so I ran a negative control — appending one over-length
unwrapped line to
TODO.mdmakesprettier --checkfail on that file, andremoving it restores the pass. The green is real and
'**/*.md'genuinelycovers
TODO.md. Worktree restored clean.Everything else
Single commit; merge-base is current
main821a293and a dry merge is clean(
mergeable: true); two files vsmain, no scope creep and no debris;git statusclean at the head; nothing stranded — the remote branch is exactly5f998c6and the PR is self-contained, needing no unpushed or uncommitted work.No Claude or Anthropic reference anywhere in tree, diff, commit message,
trailers, PR body or comments — the only matches are the pre-existing
.claude/path entries in.gitignore/.dockerignore/TODO.md, allpresent on
mainbefore this branch and none of them attribution prose.The PR body still states plainly that Cloudflare Pages parsing is unverifiable
pre-merge, names
strict-transport-security/content-security-policyas theheaders to check, and calls out
x-content-type-optionsas a false pass. Itdoes not overstate.
Disclosures and anomalies
5f998c6isAuthor: clawbot <clawbot@eeqj.de>butCommit: sneak <sneak@sneak.berlin>, where8d639aahadclawbotforboth. An artifact of the rework clone's git config, so the amend is recorded
as committed by
sneak. No policy rule inREPO_POLICIES.mdcovers commitidentity, the content is unaffected, and I am not filing it as a defect —
noting it so it is not discovered later as a surprise.
curlprobes. Noheadless browser on this host, and both concern
static/_headers, which isunchanged from the head that was already passed on the merits. Nothing in this
delta could have invalidated them.
commit subject says "security headers". Both end
(closes #14).Labels and assignment left to the caller.
PASS accepted from a fresh reviewer independent of both the author and the
first reviewer.
merge-ready, merging directly —mainis unprotected.The check that made this cheap:
git diff 8d639aa 5f998c6touchesTODO.mdonly, and the diff over
static/_headersis empty. The file the first reviewcleared on the merits is byte-identical at the candidate head, so that analysis
carries forward by construction rather than needing to be redone.
Two things worth keeping from this review:
It ran a negative control on the formatting check. A clean
prettier --checkonTODO.mdcould have been an ignore-file artifact ratherthan a real pass, so it appended an over-length unwrapped line, confirmed the
check then fails, and removed it.
'**/*.md'genuinely covers the file. Thatis the same instinct as the planted-defect control on #13 — proving the harness
can fail before trusting it to pass.
It re-ran the entity sweep itself rather than accepting the author's report,
across all five tracked Markdown files, and correctly identified the one
tree-wide hit as legitimate:
themes/loravega/layouts/index.htmluses&amp;in HTML, which is correct there and outside the diff.Anomaly disclosed, not blocking:
5f998c6recordsAuthor: clawbotbutCommit: sneak, where8d639aahadclawbotfor both. A git-config artifactof the fresh clone the rework used (the branch was already checked out in
another worktree). No policy covers commit identity and the content is
unaffected, so it is not worth a force-push to fix — noting it so the
provenance is not confusing later.
Cosmetic, not reworked: PR title says "baseline security headers", commit
subject says "security headers".
#14 stays open after this merge. Its definition of done has one step left
that cannot be done from a branch: confirming Cloudflare Pages actually parses
the file. Pages silently ignores a malformed
_headers, so nothing verified sofar speaks to it. Running that check now against both hostnames and reporting
on the issue.