Repository navigation
Discard Caddy's own diagnostics in front of the relay #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "ftw-webapp": patch | ||
| --- | ||
|
|
||
| Discard Caddy's own diagnostics in front of the relay, as the escrow already does, so a proxy or TLS error no longer writes a client address or a rendezvous handle to disk. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // @vitest-environment node | ||
|
|
||
| /* The proxy in front of the relay, held to keeping no address. | ||
| * | ||
| * Like the escrow's test, this checks configuration, which cannot prove what | ||
| * Caddy does. It catches the one line that went wrong: Caddy's own | ||
| * diagnostics went to a file with no filter, so one proxy error wrote the | ||
| * client address and the URI, which carries the rendezvous handle, beside the | ||
| * time. Go's TLS handshake errors put the address in the message text, where | ||
| * a field filter cannot reach it, so the diagnostics are discarded. | ||
| * | ||
| * The site's access log stays, because the operator reads aggregate traffic | ||
| * from it. Its filter must keep deleting the fields that name a household or | ||
| * an address. | ||
| */ | ||
|
|
||
| import { describe, it, expect } from 'vitest' | ||
| import { readFileSync } from 'node:fs' | ||
|
|
||
| const caddyfile = readFileSync(new URL('./Caddyfile', import.meta.url), 'utf8') | ||
|
|
||
| /** The body of every `log { … }` block, comments stripped. */ | ||
| function logBlocks(text: string): string[] { | ||
| const source = text.replace(/^\s*#.*$/gm, '') | ||
| const blocks: string[] = [] | ||
| const opener = /(^|\s)log\s*\{/g | ||
| let match: RegExpExecArray | null | ||
| while ((match = opener.exec(source))) { | ||
| let depth = 1 | ||
| let at = opener.lastIndex | ||
| while (at < source.length && depth > 0) { | ||
| if (source[at] === '{') depth++ | ||
| if (source[at] === '}') depth-- | ||
| at++ | ||
| } | ||
| blocks.push(source.slice(opener.lastIndex, at - 1).trim()) | ||
| } | ||
| return blocks | ||
| } | ||
|
|
||
| describe('the proxy in front of the relay', () => { | ||
| const blocks = logBlocks(caddyfile) | ||
| const global = blocks[0] ?? '' | ||
| const site = blocks[1] ?? '' | ||
|
|
||
| it('has exactly two log blocks: the global one and the site access log', () => { | ||
| // Without the global block the default logger goes back to stderr, | ||
| // where Docker keeps it. A third block needs its own rule here. | ||
| expect(blocks.length, 'expected the global log and the site access log, and nothing else').toBe(2) | ||
| }) | ||
|
|
||
| it("discards Caddy's own diagnostics", () => { | ||
| expect(global.split('\n').map((line) => line.trim()).filter(Boolean)).toEqual(['output discard']) | ||
| }) | ||
|
|
||
| it('deletes the handle, the headers and every address from the access log', () => { | ||
| for (const field of ['request>uri delete', 'request>headers delete', 'request>remote_ip delete', 'request>client_ip delete']) { | ||
| expect(site, `the access log keeps ${field.split(' ')[0]}`).toContain(field) | ||
| } | ||
| }) | ||
|
|
||
| it('never writes a log to stdout or stderr, where Docker keeps it', () => { | ||
| expect(caddyfile).not.toContain('output stdout') | ||
| expect(caddyfile).not.toContain('output stderr') | ||
| }) | ||
| }) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an operator adopts this command as the replacement for the discarded diagnostics, it never receives the health endpoint's successful response:
curl --manualdocuments that HTTP-IusesHEAD, whileRelayServer.start()accepts/healthzonly whenreq.method === 'GET'and otherwise returns 426. This can produce a permanent false failure in monitoring; use the GET command already documented inrelay/deploy/README.mdor add HEAD handling to the endpoint.Useful? React with 👍 / 👎.