Three leftovers: the /dicom block stops logging on the server, the verify suite probes a route that exists, and the stdin guard sees a redirect - #387
Merged
Conversation
…y suite probes a route that exists, and the stdin guard sees a redirect Three leftovers from the 3.1 deploy rounds, each with its own gate. 1. Phase 5c gets a sixth edit: access_log off in the server block on listen 127.0.0.1:8090, the one that backs /dicom/. #374 put the line in deploy/nginx-paramant-live.conf and wrote "repo only, so far" above it, because 5c edits the live confs by anchor and never copies a repo file over them, so the line did not travel with a deploy. It was the only block in that file still writing a combined line, and /security claims logging is off on every block that serves the site. The anchor is the listen line, because this block carries neither a server_name nor the ParaID deny the other five edits key on. Two passes, like the buffers and the /pararules 301: pass one learns which of those blocks already have the line, pass two inserts only into the ones that do not. Counted per block before and after, added to the pending count, and asserted afterwards: a :8090 block left logging stops the deploy. What it was recording is the request line, the URI, the timing, the status, the referrer and the user-agent of each call. Not the client IP: set_real_ip_from lives in the :8080 block and location /dicom/ forwards no X-Real-IP, so $remote_addr here is 127.0.0.1 on every request. 2. scripts/post-deploy-verify.sh probed /health/deep, a path the relay has never served. The route is /v2/health/deep. That check answered 404 on every run, so the suite could never exit 0, and phase 6g waved it away with a warn calling the probe "known red". A permanently red check is a check nobody reads. The probe now asks /v2/health/deep and expects the 401 the #322 gate gives a caller with no token, which is the strongest thing that script can prove: it reads no .env and takes no secret on its command line. The authenticated 200 stays phase 6c, which has the token. With exit 0 reachable, 6g is hard: any non-zero exit stops the deploy. 3. The stdin guard in tests/deploy-3.1-dryrun.test.sh accepted a command as bare only when what followed it was the end of the segment or one of ; & ) and a backtick. An output redirect is none of those, so `cat > /tmp/x` and `sort > /tmp/y` walked straight through while both eat the rest of the script exactly as a lone cat does. > is now in the closing class. `cat file > out` still does not match, because the operand sits between the command and the redirect, which is the distinction the function measures. Gates. The dry-run suite goes from 332 to 376 checks, all green: 6g-5 runs the real 5c block against a live conf whose :8090 block lacks the line and asserts the insert lands under the listen line and nowhere else, 6g-6 runs it again and asserts both confs come out byte-identical, 6g-7 does the same on a conf that had the line from the start, 6k-1 runs the stdin scan over a fixture written to break it, and 6n pins the verify route and the hard 6g. The awk was also run over the real deploy/nginx-paramant-live.conf with the line stripped out: it reproduces the file byte for byte, a second pass changes nothing, and the public conf is untouched. Also: static-sanity, check-test-declarations, shellcheck -S error, check-commit-style, tests/site-claims.test.mjs.
…ile is refused, and the block anchor's limit is written down Three things the reviewer of #387 asked for before the next real deploy. 1. post-deploy-verify.sh had no retries, and curl writes http_code 000 on every transport failure: DNS did not resolve, connection refused or reset, TLS did not come up, deadline passed. Now that 6g is hard, one CDN hiccup, one 429 or one DNS blip would stop a completely healthy deploy, and it would stop it after the work landed and before 6h, 6i and the phase 7a marker. That is deploy run 6 all over again, and the missing marker is what stops the next run in 1a. The shared curl now carries --retry 2 --retry-delay 2 --retry-all-errors, which is what makes --retry cover a transport error and not only a status curl considers transient. Around it, http_code asks once more from scratch when the answer is still 000, because every attempt inside that one curl has already been spent by then. A real status is never retried: a genuine 503 stays a 503 and does not become a slow one. 2. Phase 5c counts, and refuses, a :8090 block that writes its access log to a FILE. `access_log off;` above an `access_log /var/log/nginx/dicom.log;` leaves two access_log directives on the same level and undoes a log somebody put there on purpose. Neither is a deploy's call. The counter `before 8090 blocks logging to a file` is printed, a non-zero value is FATAL before anything is written, and the edit awk carries the same guard as a second lock. 3. DEPLOY-3.1.md says what the block anchor does not see. Both block-counting edits, the /pararules 301 and this one, walk the conf on ^server[[:space:]]*{, which is pinned to column 0. An indented `server {` is invisible to the walk. That is safe rather than wrong, an unseen block is never written into, but the counts are the thing to read if a conf ever indents its blocks. Gates. Dry-run suite 376 to 390 checks, all green. 6n-1 runs the real http_code() with curl stubbed: a 000 then a 200 answers 200 in exactly two attempts, two 000s answer 000 and stop at two attempts, and a 503 is reported on the first attempt and never retried into a 200. 6g-8 runs the real 5c block against a :8090 block that logs to a file: counted as 1, refused by name, conf byte-identical afterwards, and the edit awk on its own leaves it alone. Sabotages, both directions: retry loop stripped (3 FAIL), retry widened to any non-200 (1 FAIL, the 503 case), curl retry flags dropped (1 FAIL), the FATAL removed (1 FAIL), the awk guard removed as well (1 FAIL), the file counter pinned to zero (2 FAIL). Also: static-sanity, check-test-declarations, shellcheck -S error, check-commit-style. The edit awk still reproduces deploy/nginx-paramant-live.conf byte for byte from a copy with the line stripped out.
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
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.
Three restjes from the 3.1 deploy rounds, one PR, each with a gate that goes red without it.
1. Phase 5c gets a sixth edit:
access_log offon the:8090block#374 put the line in the
:8090server block ofdeploy/nginx-paramant-live.confand wrote repo only, so far above it, because 5c edits the live confs by anchor and never copies a repo file over them. Without an edit of its own the line does not travel with a deploy, and that block was the only one in the file still writing a combined line while /security claims logging is off on every block that serves the site.The anchor is the
listen 127.0.0.1:8090;line, because this block carries neither aserver_namenor the ParaID deny the other five edits key on. Two passes, like the buffers and the/pararules301: pass one learns which of those blocks already carry the line, pass two inserts only into the ones that do not. Counted per block before and after, added to the pending count, and asserted afterwards, so a:8090block left logging stops the deploy.Be exact about what it was recording: the request line, the URI, the timing, the status, the referrer and the user-agent of each
/dicom/call. Not the client IP.set_real_ip_fromlives in the:8080block andlocation /dicom/forwards noX-Real-IP, so$remote_addrhere is127.0.0.1on every request.docs/site-claims.mdand the note above the block no longer say repo only.2.
post-deploy-verify.shprobed a path the relay has never servedThe route is
/v2/health/deep, not/health/deep. That check answered 404 on every run, the suite could never exit 0, and phase 6g swallowed it with awarncalling the probe "known red". A permanently red non-critical check is a check nobody reads, and it was the only thing between 6g and a hard verdict.The probe now asks
/v2/health/deepand expects 401: outsideRELAY_MODE=fullthe route sits behindX-Internal-Auth(#322) and this script holds no token, so a shut gate is the strongest thing it can prove. A 200 without a token would mean the gate is open, and that is worth a FAIL. The authenticated 200 stays phase 6c, which does have the token. With exit 0 reachable, 6g is hard: any non-zero exit stops the deploy.3. The stdin guard let
cat > /tmp/xthroughbare()accepted a command as bare only when what followed was the end of the segment or one of;&)and a backtick. An output redirect is none of those, socat > /tmp/xandsort > /tmp/yread as "cat with an operand" and walked through, while both eat the rest of the remote script exactly as a lonecatdoes.>is now in the closing class.cat file > outstill does not match, because the operand sits between the command and the redirect, which is the distinction the function measures.Evidence
Dry-run suite 332 to 376 checks, 0 failed. New sections:
6g-5runs the real 5c remote block against a live conf whose:8090block lacks the line and a public conf that has no such block: insert lands, exactly one newaccess_log off, directly under the listen line, the public conf byte-identical,before edits pending = 1.6g-6runs it again over the same confs:already applied = yes,after edited files = 0, both confs byte-identical, so the diff is empty.6g-7does the same on a conf that had the line from the start (a hand edit): nothing rewritten, still tests and reloads nginx.6k-1runs the stdin scan over a fixture written to break it. Two findings expected and got,cat file > out,sort file > out,cat "$f" > "$g",cat </dev/null,wc -l < "$f"and a pipedcatall left alone.6npins the verify route, the 401 expectation, that no runnable line names the old path, and that both non-zero branches of the 6g verdict aredie.Against the real conf, not just fixtures: the edit awk run over
deploy/nginx-paramant-live.confwith theaccess_log offline stripped from the:8090block reproduces the file byte for byte, a second pass changes nothing, anddeploy/nginx-paramant-public.confcomes out untouched.Sabotages, both directions per fix:
!has/!insguard)/health/deepand the 200 expectation backdieback into awarn($|[;&)])`cat file > outand friends now flagged, plus the real scan goes redOther gates:
tests/static-sanity.shPASS,scripts/check-test-declarations.sh118 suites clean,shellcheck -S errorclean on all three scripts (no new warning at-S warningeither),scripts/check-commit-style.shclean,node --test tests/site-claims.test.mjs36/36.tests/heartbeat-lib.test.mjsfails ona525070too, before this branch touches anything.deploy/DEPLOY-3.1.mdis updated: 5c is six edits, the:8090edit has its own section with the anchor and what the log did and did not hold, and the 6g paragraph says exit 0 is now reachable and any non-zero exit stops the deploy.Review round (commit
919ba8d6)Must, before the next real deploy: the probes retry.
post-deploy-verify.shhad none, and curl writeshttp_code000on every transport failure. With 6g hard, one CDN hiccup, one 429 or one DNS blip would stop a healthy deploy after the work landed and before 6h, 6i and the 7a marker, which is run 6 all over again. The shared curl now carries--retry 2 --retry-delay 2 --retry-all-errors, andhttp_codeasks once more from scratch when the answer is still000(every attempt inside that one curl is already spent by then). A real status is never retried, so a genuine 503 stays a 503.A
:8090block that logs to a FILE is refused, not stacked. New counterbefore 8090 blocks logging to a file; non-zero is FATAL before anything is written, and the edit awk carries the same guard as a second lock.access_log off;above anaccess_log /var/log/nginx/dicom.log;leaves two directives on one level and undoes a log somebody put there on purpose.The anchor's limit is written down.
DEPLOY-3.1.mdnow says that both block-counting edits walk on^server[[:space:]]*{, pinned to column 0, so an indentedserver {is invisible to the walk. Safe rather than wrong (an unseen block is never written into), but the counts are the thing to read if a conf ever indents its blocks.Dry-run suite 376 to 390, all green.
6n-1runs the realhttp_code()with curl stubbed:000then200answers 200 in exactly two attempts; two000s answer000and stop at two; a503is reported on the first attempt and never retried into a 200.6g-8runs the real 5c block against a file-logging:8090block: counted as 1, refused by name, conf byte-identical afterwards, and the edit awk on its own leaves it alone. The plain fixtures read the counter as 0, soaccess_log offand "no access_log at all" do not read as file logging.http_code