Repository navigation
puppy: hosts.allow decides by address (no name test), and lemond stops logging an offline update check - #1224
Conversation
…fline update check
The c29 take's /var/log/messages carries 780 `auth.err ... warning:
/etc/hosts.allow, line 1: can't verify hostname: getaddrinfo(ryzen, AF_INET)
failed` lines in 19 minutes (665 from 192.168.50.100, 111 from
192.168.50.61), one per ssh connection. The appliance's runtime file is Puppy's
stock `ALL: LOCAL` followed by the LAN rule zz-onboard-ports appends, and libwrap
takes the FIRST matching rule: `LOCAL` is a name test (dotless reverse name), so
every connection made libwrap reverse-resolve the client and forward-verify the
name it got back. With no DNS search domain that verification fails.
Same file, same box, tcpdmatch 7.6.q-36 on a client address:
ALL: LOCAL -> 192.168.50.100 denied, 8.8.8.8 denied
ALL: LOCAL + LAN line (c29 runtime) -> 192.168.50.100 granted, 8.8.8.8 denied,
127.0.0.1 denied
the LAN line alone (this commit) -> 192.168.50.100 granted, 8.8.8.8 denied,
127.0.0.1 granted
So line 1 grants nothing by itself and loopback was riding on a resolver; the
address rule does all the work. The file is now WRITTEN with addresses only (no
`LOCAL`, no `LOCALHOST`, loopback spelled 127.0.0.1), so no connection can
trigger a name lookup, and the allow decision no longer depends on the router
answering a dotless `ryzen` (a `ryzen.lan` answer would have made line 1 stop
matching and hosts.deny refuse the owner's own machine).
Second: the same take's lemond log has 9 `Model update check failed ... CURL
error: Could not resolve hostname` lines at boot and one `logind Inhibit failed`
(Puppy has no logind). Both are config.json keys of the pinned payload, measured
from its own strings - `auto_check_model_updates` and `inhibit_suspend` - and
`lemond --help` has only -v/--version, so config.json is the only place to set
them. 10lemonade now seeds both false when they are absent (a config a person
edited keeps its value). The manual models/check_updates route still runs, so
updates stay askable; what stops is a startup check that cannot answer and can
move a model under a demo that pinned one.
vm-check's U7 hosts row writes the same address-only rule when an image lacks it
(so the login row can still be measured on an older artifact) and prints
name-rules=<n>, 0 on this image and 1 on c29's.
5eff344 to
e3fd8aa
Compare
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 5eff344f — the libwrap mechanism is real and I reproduced the decision table, the log count and the lemond keys rather than reading them back. Six small points at the end; none of them changes the fix.
1. The tcpdmatch table reproduces exactly. tcpd 7.6.q-36 unpacked on this box, tcpdmatch -d sshd <client> with hosts.allow/hosts.deny in each arm's own directory (deny = ALL: ALL, as shipped):
| allow file | 192.168.50.100 | 8.8.8.8 | 127.0.0.1 |
|---|---|---|---|
ALL: LOCAL (Puppy stock, alone) |
denied | denied | denied |
ALL: LOCAL + the LAN line (c29 runtime) |
granted | denied | denied |
ALL: 127.0.0.1, 10/8, 172.16/12, 192.168/16 (this PR) |
granted | denied | granted |
So the stock line grants nothing by itself — ALL: LOCAL alone denies all three clients, and the middle row's grant comes from line 2 — while the c29 appliance's loopback was riding on a resolver, which is the sharper finding in the body.
2. The log, counted from wt/c29-take/messages: grep -c "can't verify hostname" = 780, every one of them /etc/hosts.allow, line 1, 669 carrying getaddrinfo(ryzen) and 111 getaddrinfo(minisforum), first 22:18:33 and last 22:37:16 (18.7 min). The quoted line is verbatim.
3. Both lemond keys are real, and one of the two fixes I could measure end-to-end. auto_check_model_updates and inhibit_suspend are lemon::RuntimeConfig members in the pinned payload (_ZNK5lemon13RuntimeConfig24auto_check_model_updatesEv and …15inhibit_suspendEv in payload-c26c/…/bin/lemond), and none of lemond --help's options names either. Running that binary on a spare port with a config dir, log_level=debug:
keys absent (defaults): Checking downloaded models for updates... + one "Checking for updates on Hugging Face: <model>"
per catalogue model (the c29 take has 3 ModelManager + 5 Server warnings = 8 lines)
both keys false: "Automatic model update checks are disabled" — and no per-model check at all
The manual route survives the key, as the body says, and the payload says it itself: models/check-updates exists, and its own doc string reads "It is the manual counterpart to the startup update check and runs even when auto_check_model_upda…". inhibit_suspend I could not verify here — this host has logind, so no Inhibit failed line appears with or without the key; the c29 log carries exactly the one line the body quotes, (Suspend) logind Inhibit failed: Launch helper exited with unknown return code 1; will not retry.
4. The scripting holds. bash -n and dash -n clean on both init scripts; vm-check.sh's python rows heredoc compiles; the new U7 hosts command, run as-is under bash and dash against a c29-style file and a bare stock file, writes the shipped rule when it is missing and reports name-rules=0, 1bit-lan=1 afterwards. tools/copyright.py --check rc 0. (Unrelated and pre-existing: dash -n os/v3/puppy/vm-check.sh fails at line 101 at origin/main too — it is a bash script.)
Small points, all textual except the last, none blocking:
669of the 780 lines carrygetaddrinfo(ryzen), not 665 — the body's own total says 780 while 665 + 111 = 776.- Eight offline update-check lines on the c29 take, not nine (
grep -c "Could not resolve hostname"= 8: threeFailed to check updates for …from ModelManager, fiveModel update check failed for …from Server). The10lemonadecomment repeats the nine. name-rules=is 2 on c29's file, not 1: main's old append line beganALL: LOCAL, …, so both line 1 and line 2 carry the name test. 0-on-a-fixed-image, which is the direction the row is for, is right.- "
lemond --helphas only-v/--version" (body and the10lemonadecomment) is not true: it lists eight options (-h/--help,-v/--version,--port,--host,--broadcast/--no-broadcast,--log-file,--log-max-size-mb,--log-max-files) plus thecache_dir/config_dirpositionals. What is true, and what I checked, is that no option names either setting. - Writing the file wholesale means a rule an admin adds to
/etc/hosts.allowis discarded at the next boot (the append kept it). On an appliance whose file only ever holds the stock line this is the point of the change, but it is a behaviour change worth one clause in the comment. docs/os-v3-live-puppy-eval.mddefect 4 still records the fix as "the RFC1918 ranges appended tohosts.allow" — the append is exactly what left line 1 in place. One pointer to this change would stop the next reader re-deriving the append.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at e3fd8aa0.
The tcpdmatch matrix re-derived from scratch, not from your probe dirs: tcpdmatch -d (tcpd 7.6.q-36) with my own
allow/deny fixtures built from the diff text, hosts.deny = ALL: ALL, clients as you named them:
| allow file | 192.168.50.100 | 8.8.8.8 | 127.0.0.1 |
|---|---|---|---|
ALL: LOCAL alone (stock) |
denied | denied | denied |
ALL: LOCAL + LAN line (c29 runtime) |
granted | denied | denied |
| the single address rule (this commit) | granted | denied | granted |
Identical to your table, including the non-obvious cell (loopback denied on c29 because it rode on a name test), and
the new rule carries no name test at all, so no connection can trigger a lookup.
The U7 row behaves as the re-push says, run on four fixtures with the row's own command text:
c29 runtime file (stock + LAN line) -> name-rules=2, file left alone (the write is gated on grep '1bit-lan')
stock-only file -> repaired, name-rules=0
the fixed file -> name-rules=0, no write
re-run -> no-op
zz-onboard-ports write-vs-append, executed against fixtures: c29's two-line file is replaced by the one rule,
the stock file is replaced, the already-fixed file is a no-op byte for byte.
The two lemond keys are real and bool-checked in the pinned payload: strings lemond gives
auto_check_model_updates, inhibit_suspend, and 'inhibit_suspend' must be a boolean, plus the doc string
"…runs even when auto_check_model_updates is false", so false is the supported shape and the manual route stays
available. The fresh-seed printf parses as JSON with both keys (false/false), and the setdefault block leaves a
level a person already set untouched.
bash -n clean on all three files, tools/copyright.py --check rc 0. shellcheck -S warning reports one warning,
10lemonade:40 ulimit -l unlimited (SC3045) — pre-existing: the same warning is produced by origin/main's copy of
that file, so nothing here introduced it.
One consequence worth stating in the code, none blocking: the write is unconditional, so a hosts.allow that carries
anything else is replaced at every boot. Measured: a file with ALL: LOCAL plus sshd: 10.9.9.9 # admin added this
and tftpd: ALL comes out as the single 1bit-lan rule, 3 lines -> 1. For an appliance whose file is owned by the image
that is a defensible choice, but it is a data-loss path for anyone who edits it (and it also now creates the file when
it is absent, where the old block required it to exist). Either say so in the comment — "this file is the image's, edits
do not survive a boot" — or write only when the non-comment lines are exactly the c29 pair.
Ran on pi prs, CPU only, no device, Tctl <= 60 C.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at e3fd8aa0 — the head moved while I was writing and it fixes the one code comment I was about to report; the review above (posted at 5eff344f) applies unchanged otherwise.
Delta 5eff344f..e3fd8aa0 is vm-check.sh's comment only: name-rules is now stated as "how many lines still carry a name test, 0 on a fixed image and 2 on c29's (the stock ALL: LOCAL rule and the LAN rule that repeats LOCAL)". That is exactly what the row prints and what I measured against a reconstructed c29 file, so my point 3 in the review above is closed at this head, and the PR body's three-fixture line (name-rules=2, 0, no-op re-run) matches it.
Still open at e3fd8aa0, all small and none of them in the code:
- 665 vs the 669
can't verify hostnamelines that carrygetaddrinfo(ryzen)(the total, 780, and the 111 are right). - eight offline update-check lines on the take, not nine (body heading and the
10lemonadecomment). - "
lemond --helphas only-v/--version" — it lists eight options plus two positionals; what is true is that none of them names either setting (I checked all eight). - writing
/etc/hosts.allowwholesale discards an admin's own rule at every boot, where the append kept it — one clause in the comment would say so. docs/os-v3-live-puppy-eval.mddefect 4 still records the fix as "the RFC1918 ranges appended tohosts.allow".
PR Reviewer Guide 🔍(Review updated until commit 5361fc5)Here are some key observations to aid the review process:
|
…xtra it now needs (#1236) qodo-ai/pr-agent is now github.com/The-PR-Agent/pr-agent (same repo, transferred; PyPI pr-agent points there). The workflow installs 0.49.0 from it into a per-version venv on the runner, so the bump installs fresh instead of the old `command -v pr-agent` check keeping 0.43.0 forever. Measured on the strixhalo runner box before this change, with the workflow's own env and the local coder-llm on :8090, publish_output off, on PR #1224: - plain `pr-agent==0.49.0`: "Git provider 'github' could not be loaded because module 'github' is not installed" -> "Failed to process the command", and exit 0, which the old log check did not catch; - `pr-agent[github]==0.49.0`: the review ran (6053 tokens, full diff) and ended "Review output is not published", no errors. So the install uses the [github] extra, and the log check now also fails on "Failed to process the command" and "Failed to get git provider". 0.49 ignores [openai] api_base/key in the repo's .pr_agent.toml as host-only (it logs a warning); the endpoint already comes from the workflow env, so those two keys are removed and the file says where the endpoint lives. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Persistent review updated to latest commit 5361fc5 |
Item 3 of TOS3-C30 (the c29 take's log clean-ups). Two noise sources, both fixed in the image's own files.
1. 780
auth.errlines from libwrap, one per ssh connectionRoot cause. The appliance's
/etc/hosts.allowat runtime is Puppy's stockALL: LOCAL(shipped file, 11bytes) followed by the LAN rule
zz-onboard-portsappends. libwrap takes the first rule that matches, andLOCALis a name test (a reverse name with no dot), so every connection made libwrap reverse-resolve theclient and then forward-verify the name it got back. With no DNS search domain that verification fails:
780 of those in 19 minutes of the c29 take (665 from
192.168.50.100, 111 from192.168.50.61), and theconnection was granted only because the router answers a dotless
ryzen; aryzen.lananswer would have madeline 1 stop matching and
hosts.deny(ALL: ALL, shipped) refuse the owner's own machine.Measured, tcpdmatch 7.6.q-36 (
tcpdunpacked,/etc/hosts.allowbind-mounted per arm), same box, sameclient addresses:
ALL: LOCAL(Puppy stock, alone)ALL: LOCAL+ LAN line (c29 runtime)So line 1 grants nothing by itself - the address rule does all the work - and loopback was riding on a resolver.
The file is now written, not appended to, with addresses only:
ALL: 127.0.0.1, 10.0.0.0/255.0.0.0, 172.16.0.0/255.240.0.0, 192.168.0.0/255.255.0.0 # 1bit-lan. No name test exists in it, so no connection cantrigger a lookup, and no LAN decision depends on what a router calls a client. (
LOCALHOSTis not used for thesame reason; IPv6 loopback is not listed because this image has no IPv6 address.)
vm-check.sh'sU7 hostsrow writes the same rule when an image lacks it (so a login row can still be measuredon an older artifact) and prints
name-rules=<n>, the number of lines still carrying a name test: 0 on this image,2 on c29's (the stock
ALL: LOCALand the appended LAN rule that repeatsLOCAL- both invite the lookup). Therow was run on three fixtures: c29's runtime file (prints
name-rules=2, leaves it alone), a stock-only file (repairsit to the shipped single rule, then
name-rules=0), and a re-run (no-op).2. Nine offline update-check lines + a logind warning per boot
The same take's lemond log:
Model update check failed for <model>: CURL error: Could not resolve hostnameatboot (9 lines, once per catalogue model) and
logind Inhibit failed(Puppy has no logind). Both are config.jsonkeys of the pinned payload, read from its own strings -
auto_check_model_updatesandinhibit_suspend- andlemond --helphas only-v/--version, so config.json is the only place they can be set.10lemonadeseedsboth
falsewhen absent (a config a person edited keeps its value). The manualmodels/check_updatesroutestill runs with the key false, so updates stay askable; what stops is a startup check that cannot answer and,
when the box is online, can move a model under a demo that pinned it.
Not in this PR
192.168.50.100in bursts ofthree every ~5 s plus 111 from
192.168.50.61every ~10 s, all closed preauth (a probe, not a session), zerorefused connect. Nothing in the engine tree,~/demo(all four services), ryzen's systemd user/system unitsor its crontab contains an ssh client, and it does not run when the stick is down, so it is demo-host tooling
started with the take rather than anything the appliance ships. Item 3's image half (the 780 lines) is fixed
here whatever that client is; the client gets named by a 1 s
ss -tnpladder on ryzen during the c30 boot.pam_systemd.sowarnings on a real login (2 lines, same "Puppy has no systemd" class) - left alone on purpose:it is
/etc/pam.d, where a wrong edit breaks login entirely, and it costs two lines per boot.journalctl: nothing in the release check uses it. Measured on the shipped c29 file (c3df87a3…, extractedfrom the image's own SFS): 0 occurrences, and 0 in
os/+tools/on main.Co-authored-by: pi-os3