fix(hub-settings): setter verloor stil de eigenaar van het envbestand #147

Merged
janpeter merged 2 commits from fix/hub-setter-ownership into main 2026-08-20 10:06:57 +02:00
Owner

Twee defecten, allebei gevonden doordat de veldproef op scrum4me-server het schrijfpad weerlegde in plaats van bevestigde. PR #143 was live en groen; dit is wat er onder zat.

1. De setter verloor stil de eigenaar van het envbestand

De proef zette HUB_HOOK_WAIT_SECONDS op zijn eigen waarde 120. De inhoud bleef byte-identiek, maar de eigenaar ging van janpeter:janpeter naar root:root. De permission-hook draait als janpeter en kon zijn eigen envbestand daarna niet meer lezen; de bekende faalmodus is dan dat de hook exit 0 geeft zonder decision, waarna approvals stil buiten de hub om lopen. Eén klik op /settings/hub schakelde de hub dus uit.

Het naarste eraan: de degradatie was in beide richtingen stil. Het leespad bleef werken omdat de hartslag via sudo als root leest — het dashboard bleef groen terwijl de hook uit stond.

Oorzaak. De ownership-regel stond op BSD-eerst:

OWNER=$(stat -f '%u:%g' "$F" 2>/dev/null || stat -c '%u:%g' "$F" 2>/dev/null || true)

Op GNU coreutils is stat -f filesystem-status en neemt het geen format-argument: '%u:%g' wordt daar als bestandsnaam gelezen. Het commando faalt, maar heeft dan al vijf regels filesystem-info naar stdout geschreven, en die belanden in de command-substitution. De fallback plakt er 1000:1000 onder, chown krijgt een string van zes regels, faalt — en die fout werd door 2>/dev/null || true opgeslokt.

Op macOS werkt stat -f juist wél. Lokaal was dit dus onzichtbaar, en de bestaande tests toetsten na de rewrite wel de mode maar niet de eigenaar. Zo is het door elf groene tests geglipt.

Fix: GNU eerst (BSD kent geen -c en faalt schoon), een numerieke guard tegen vervuilde uitvoer, en geen || true meer — een mislukte chown faalt nu hard, waarna de EXIT-trap het tempbestand opruimt en het origineel ongemoeid blijft. De nieuwe test stubt stat in beide dialecten plus chown en eist één schone numerieke eigenaar.

2. setup.sh ontwapende de hub-module bij elke run

deploy/ops-agent/setup.sh overschrijft /etc/sudoers.d/ops-agent hard vanuit deploy/ops-agent/sudoers (nul hub-regels) en riep daarna wél install-network-module.sh en install-docker-inspection-module.sh na — maar niet install-hub-module.sh. commands.yml overleeft, dus de hub-keys bleven staan terwijl de sudoers-regels eronder verdwenen: de agent voert de key uit en sudo weigert hem. Dat raakt ook het leespad, want hub_hook_status loopt over dezelfde regel.

De guard is fail-closed op de map: elke install-*-module.sh moet door setup.sh worden aangeroepen. De control-room-harness hield diezelfde lijst handmatig bij en driftte dus op dezelfde manier — die leest de map nu ook.

Gate

tsc --noEmit groen · npm test 1186 groen. De 4 falende tests in test/scrum4us-deploy-trigger.test.ts zijn pre-existing op main. Beide nieuwe guards zijn apart rood gemaakt om te bewijzen dat ze aanslaan.

De host is intussen hersteld: envbestand terug op janpeter:janpeter 600, zelfde hash, leesbaar als janpeter, geen tempbestanden achtergebleven.

🤖 Generated with Claude Code

Twee defecten, allebei gevonden doordat de veldproef op scrum4me-server het schrijfpad **weerlegde** in plaats van bevestigde. PR #143 was live en groen; dit is wat er onder zat. ## 1. De setter verloor stil de eigenaar van het envbestand De proef zette `HUB_HOOK_WAIT_SECONDS` op zijn eigen waarde 120. De inhoud bleef byte-identiek, maar de eigenaar ging van `janpeter:janpeter` naar `root:root`. De permission-hook draait als `janpeter` en kon zijn eigen envbestand daarna niet meer lezen; de bekende faalmodus is dan dat de hook exit 0 geeft **zonder decision**, waarna approvals stil buiten de hub om lopen. Eén klik op `/settings/hub` schakelde de hub dus uit. Het naarste eraan: de degradatie was in beide richtingen stil. Het leespad bleef werken omdat de hartslag via `sudo` als root leest — het dashboard bleef groen terwijl de hook uit stond. **Oorzaak.** De ownership-regel stond op BSD-eerst: ```bash OWNER=$(stat -f '%u:%g' "$F" 2>/dev/null || stat -c '%u:%g' "$F" 2>/dev/null || true) ``` Op GNU coreutils is `stat -f` *filesystem*-status en neemt het geen format-argument: `'%u:%g'` wordt daar als bestandsnaam gelezen. Het commando faalt, maar heeft dan al vijf regels filesystem-info naar stdout geschreven, en die belanden in de command-substitution. De fallback plakt er `1000:1000` onder, `chown` krijgt een string van zes regels, faalt — en die fout werd door `2>/dev/null || true` opgeslokt. Op macOS werkt `stat -f` juist wél. Lokaal was dit dus onzichtbaar, en de bestaande tests toetsten na de rewrite wel de *mode* maar niet de *eigenaar*. Zo is het door elf groene tests geglipt. **Fix:** GNU eerst (BSD kent geen `-c` en faalt schoon), een numerieke guard tegen vervuilde uitvoer, en geen `|| true` meer — een mislukte `chown` faalt nu hard, waarna de EXIT-trap het tempbestand opruimt en het origineel ongemoeid blijft. De nieuwe test stubt `stat` in beide dialecten plus `chown` en eist één schone numerieke eigenaar. ## 2. `setup.sh` ontwapende de hub-module bij elke run `deploy/ops-agent/setup.sh` overschrijft `/etc/sudoers.d/ops-agent` hard vanuit `deploy/ops-agent/sudoers` (nul hub-regels) en riep daarna wél `install-network-module.sh` en `install-docker-inspection-module.sh` na — maar niet `install-hub-module.sh`. `commands.yml` overleeft, dus de hub-**keys** bleven staan terwijl de sudoers-regels eronder verdwenen: de agent voert de key uit en `sudo` weigert hem. Dat raakt ook het leespad, want `hub_hook_status` loopt over dezelfde regel. De guard is fail-closed op de map: elke `install-*-module.sh` moet door `setup.sh` worden aangeroepen. De control-room-harness hield diezelfde lijst handmatig bij en driftte dus op dezelfde manier — die leest de map nu ook. ## Gate `tsc --noEmit` groen · `npm test` 1186 groen. De 4 falende tests in `test/scrum4us-deploy-trigger.test.ts` zijn pre-existing op `main`. Beide nieuwe guards zijn apart rood gemaakt om te bewijzen dat ze aanslaan. De host is intussen hersteld: envbestand terug op `janpeter:janpeter 600`, zelfde hash, leesbaar als `janpeter`, geen tempbestanden achtergebleven. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(hub-settings): setter verloor stil de eigenaar van het envbestand
All checks were successful
CI / Root app checks (pull_request) Successful in 5m50s
CI / Ops-agent checks (pull_request) Successful in 15s
CI / Deploy artifact checks (pull_request) Successful in 12s
CI / Docker image build (pull_request) Successful in 1m16s
2611c9f894
Gevonden door de veldproef op scrum4me-server. Die proef zette HUB_HOOK_WAIT_SECONDS
op zijn eigen waarde 120 — inhoud bleef byte-identiek, maar de EIGENAAR ging van
janpeter:janpeter naar root:root. Daarmee kon de permission-hook, die als janpeter
draait, zijn eigen envbestand niet meer lezen; de bekende faalmodus is dan dat de hook
exit 0 geeft zonder decision en approvals stil buiten de hub om lopen. Eén klik op
/settings/hub schakelde de hub dus uit.

De degradatie was in beide richtingen stil: het leespad bleef werken omdat de hartslag
via sudo als root leest, dus het dashboard bleef groen terwijl de hook uit stond.

OORZAAK. De ownership-regel stond op BSD-eerst:
  OWNER=$(stat -f '%u:%g' "$F" 2>/dev/null || stat -c '%u:%g' "$F" 2>/dev/null || true)
Op GNU coreutils is `stat -f` FILESYSTEM-status en neemt het geen format-argument:
'%u:%g' wordt daar als BESTANDSNAAM gelezen. Het commando faalt, maar heeft dan al vijf
regels filesystem-info naar stdout geschreven, en die belanden in de
command-substitution. De fallback plakt er "1000:1000" onder, chown krijgt een string
van zes regels, faalt, en die fout werd door `2>/dev/null || true` opgeslokt.

Op macOS werkt `stat -f` juist wel. Ik heb dit dus lokaal nooit kunnen zien, en de
bestaande tests toetsten na de rewrite wél de mode maar niet de eigenaar — daarom is
het door 11 groene tests geglipt.

FIX: GNU eerst (BSD kent geen -c en faalt schoon), een numerieke guard tegen vervuilde
uitvoer, en geen `|| true` meer — een mislukte chown faalt nu hard, waarna de EXIT-trap
het tempbestand opruimt en het origineel ongemoeid blijft.

De nieuwe test stubt `stat` in beide dialecten plus `chown`, en eist één schone
numerieke eigenaar. Apart gecontroleerd dat hij rood wordt op de oude volgorde.

---

Tweede defect, uit dezelfde proef: deploy/ops-agent/setup.sh overschrijft
/etc/sudoers.d/ops-agent hard vanuit deploy/ops-agent/sudoers (nul hub-regels) en riep
daarna wél install-network-module.sh en install-docker-inspection-module.sh na, maar
niet install-hub-module.sh. commands.yml overleeft, dus de hub-KEYS bleven staan terwijl
de sudoers-regels eronder verdwenen: de agent voert de key uit en sudo weigert hem. Dat
raakt ook het leespad, want hub_hook_status loopt over dezelfde regel.

De guard is fail-closed op de map: elke install-*-module.sh moet door setup.sh worden
aangeroepen, dus een volgende module valt er standaard onder. De control-room-harness
hield diezelfde lijst handmatig bij en drift dus op dezelfde manier — die leest de map nu
ook.
s4m-codex-reviewer left a comment

Verdict: APPROVED

Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

Findings

  • Geen blokkerende findings gevonden.

Review-notities

  • De wijziging in ops-agent/wrappers/hub/hub-hook-wait.sh corrigeert de GNU/BSD stat-volgorde, valideert vervuilde eigenaar-output fail-closed en voorkomt stil eigendomsverlies bij de atomische rewrite.
  • De extra aanroep van deploy/ops-agent/install-hub-module.sh in deploy/ops-agent/setup.sh past bij het bestaande installer-/sudoers-patroon: setup overschrijft de hoofd-sudoers en modules die eigen sudoers-regels nodig hebben moeten daarna opnieuw idempotent worden aangebracht.
  • De nieuwe tests dekken zowel de GNU/BSD stat-val als de module-installers in setup.sh. Ik kon geen lokale test-run uitvoeren omdat er geen checkout van de repo beschikbaar is in /opt/agent; de beoordeling is daarom gebaseerd op de frozen PR-diff en productdocs.
# Verdict: APPROVED Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. ## Findings - Geen blokkerende findings gevonden. ## Review-notities - De wijziging in `ops-agent/wrappers/hub/hub-hook-wait.sh` corrigeert de GNU/BSD `stat`-volgorde, valideert vervuilde eigenaar-output fail-closed en voorkomt stil eigendomsverlies bij de atomische rewrite. - De extra aanroep van `deploy/ops-agent/install-hub-module.sh` in `deploy/ops-agent/setup.sh` past bij het bestaande installer-/sudoers-patroon: setup overschrijft de hoofd-sudoers en modules die eigen sudoers-regels nodig hebben moeten daarna opnieuw idempotent worden aangebracht. - De nieuwe tests dekken zowel de GNU/BSD `stat`-val als de module-installers in `setup.sh`. Ik kon geen lokale test-run uitvoeren omdat er geen checkout van de repo beschikbaar is in `/opt/agent`; de beoordeling is daarom gebaseerd op de frozen PR-diff en productdocs.
merge: main in fix/hub-setter-ownership
All checks were successful
CI / Root app checks (pull_request) Successful in 5m45s
CI / Ops-agent checks (pull_request) Successful in 15s
CI / Deploy artifact checks (pull_request) Successful in 12s
CI / Docker image build (pull_request) Successful in 1m16s
ee3671b87a
Twee conflicten, beide inhoudelijk:

deploy/ops-agent/setup.sh — main lost defect 2 intussen zelf op en roept
install-hub-module.sh al aan (regel 210). Mijn blok was dubbelop; main's versie
gehouden.

test/control-room-foundation-harness.ts — main voegde 'install-hub-module.sh'
toe aan de handgeschreven lijst. Dat werkt, maar het is dezelfde driftbron die
het defect veroorzaakte: de volgende module wordt in setup.sh wél aangeroepen en
hier niet gestubd, waarna setup.sh in de nep-repo faalt op een ontbrekend bestand
en de fout in de foundation lijkt te zitten. Mijn map-lezende versie gehouden;
die is fail-closed en dekt elke volgende module vanzelf.

De guard in test/ops-agent-setup-modules.test.ts blijft staan: main's fix is
correct maar niet geborgd, en deze test voorkomt dat het opnieuw gebeurt.
s4m-codex-reviewer left a comment

Verdict: COMMENT

Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

Findings

  • warning — lib/build-info.ts:3: De PR commit auto-generated build metadata mee die inhoudelijk los staat van de hub-settings fix. Bovendien wijst buildInfo.commit naar b13271f, terwijl de PR-head ee3671b87a42c5bbd4950ab88530e5b43ff259d6 is. Als deze metadata in UI/diagnostics gebruikt wordt, toont de build straks een onjuiste commit. Regeneer dit bij de daadwerkelijke release/build of laat deze wijziging buiten deze bugfix-PR.

Review

De wijziging in ops-agent/wrappers/hub/hub-hook-wait.sh adresseert de root cause: GNU stat -f kan stdout vervuilen, en de nieuwe volgorde plus validatie voorkomt dat een vervuilde owner-string stil doorloopt naar mv. De hard-fail op mislukte owner-bepaling/chown is passend fail-closed gedrag voor dit pad. De nieuwe Vitest-dekking simuleert zowel GNU als BSD stat en de fail-closed situatie, en de setup-module-test voorkomt regressie door handmatig driftende modulelijsten.

# Verdict: COMMENT Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. ## Findings - warning — `lib/build-info.ts:3`: De PR commit auto-generated build metadata mee die inhoudelijk los staat van de hub-settings fix. Bovendien wijst `buildInfo.commit` naar `b13271f`, terwijl de PR-head `ee3671b87a42c5bbd4950ab88530e5b43ff259d6` is. Als deze metadata in UI/diagnostics gebruikt wordt, toont de build straks een onjuiste commit. Regeneer dit bij de daadwerkelijke release/build of laat deze wijziging buiten deze bugfix-PR. ## Review De wijziging in `ops-agent/wrappers/hub/hub-hook-wait.sh` adresseert de root cause: GNU `stat -f` kan stdout vervuilen, en de nieuwe volgorde plus validatie voorkomt dat een vervuilde owner-string stil doorloopt naar `mv`. De hard-fail op mislukte owner-bepaling/chown is passend fail-closed gedrag voor dit pad. De nieuwe Vitest-dekking simuleert zowel GNU als BSD `stat` en de fail-closed situatie, en de setup-module-test voorkomt regressie door handmatig driftende modulelijsten.
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
janpeter/Ops-dashboard!147
No description provided.