Skip to content

fix(daemon): stop prefilling the daemon key in the templates - #1083

Merged
oleksandr-nc merged 1 commit into
mainfrom
fix/no-prefilled-daemon-key
Sep 29, 2026
Merged

oleksandr-nc merged 1 commit into
mainfrom
fix/no-prefilled-daemon-key

Conversation

@oleksandr-nc

@oleksandr-nc oleksandr-nc commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

The daemon templates in the registration form prefilled the HaRP shared key with some_very_secure_password, and the deprecated Docker Socket Proxy templates with some_secure_password or enter_haproxy_password. The field is a password field, so the value stayed hidden, and a daemon could be registered with a documentation example key without the admin ever typing or seeing it.

The templates now leave the key empty. For HaRP daemons the form's existing check keeps Register disabled until a key of at least 12 characters is entered, and the placeholder points to HP_SHARED_KEY of the HaRP container. An empty field shows the length rule as a plain hint and turns red only once a shorter key is typed, so the form no longer opens with an error. For the deprecated Docker Socket Proxy templates the password stays optional over HTTP, as before.

A spec test makes sure no template prefills a key again. The warning about example keys from #1082 stays, since the occ app_api:daemon:register --help examples and the documentation still use them.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: f0539b56-ce57-4b52-8614-f0f429ce597f

📥 Commits

Reviewing files that changed from the base of the PR and between 1f738bd and 5af1dad.

📒 Files selected for processing (3)
  • lib/Command/Daemon/RegisterDaemon.php
  • lib/Service/DaemonConfigService.php
  • tests/php/Service/DaemonConfigServiceTest.php
💤 Files with no reviewable changes (3)
  • lib/Command/Daemon/RegisterDaemon.php
  • lib/Service/DaemonConfigService.php
  • tests/php/Service/DaemonConfigServiceTest.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Six daemon templates now default haproxy_password to an empty string. A test checks this value across all templates. Secret validation no longer detects or warns about example secrets. The changelog records the removal of prefilled HaRP shared keys and HaProxy passwords.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 5af1d

Daemon templates no longer prefill example passwords, and the intended registration safeguards remain in place. No actionable merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1f738

Removing prefilled example keys reduces the chance of registering a daemon with a shared, publicly known credential. Required keys remain enforced for HaRP and HTTPS registrations. Deprecated HTTP proxy configurations can still omit a key; their effective exposure depends on how the proxy is deployed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed defaults can affect credentials chosen during new daemon registration, including registration of a deprecated proxy with no key. They do not by themselves change an existing daemon's stored key or establish that its proxy is network-exposed.

Security Findings and Attack Paths

  • inferred — No new path to persist an empty required HaRP or HTTPS key was established through the form or HTTP registration endpoint. The optional empty-key path for non-HaRP HTTP existed before this default change.

Trust Boundaries and Controls

  • observed — The form disables registration for an invalid required key. The HTTP controller requires password confirmation and validates the client-supplied secret again before calling the persistence service; the form check is not the sole control on that path.

Resilience and Maintainability Implications

  • observed — On the HTTP registration path, an invalid required key is rejected before insertion; a service insertion failure returns no configuration. The PR does not change that sequence.

Hardening Proposals

  • proposed — Consider a separate rotation path for daemons that already use documented example keys; changing template defaults does not rotate stored credentials.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: daemon templates no longer prefill the daemon key.
Description check ✅ Passed The description directly explains the template changes, validation behavior, test coverage, and removal of the example-key warning.
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@oleksandr-nc
oleksandr-nc force-pushed the fix/no-prefilled-daemon-key branch from 5af1dad to 1f738bd Compare September 29, 2026 18:37
Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
@oleksandr-nc
oleksandr-nc force-pushed the fix/no-prefilled-daemon-key branch from 1f738bd to 4a639fc Compare September 29, 2026 18:47
@oleksandr-nc
oleksandr-nc merged commit ea450cc into main Sep 29, 2026
55 checks passed
@oleksandr-nc
oleksandr-nc deleted the fix/no-prefilled-daemon-key branch September 29, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant