Create persistent notification also (not just log warning) when key expiration is detected#605
Draft
lmagyar wants to merge 11 commits into
Draft
Create persistent notification also (not just log warning) when key expiration is detected#605lmagyar wants to merge 11 commits into
lmagyar wants to merge 11 commits into
Conversation
lmagyar
added
the
enhancement
WalkthroughAdds a Home Assistant REST API Bash library and a one-time persistent notification flow in post-tailscaled/run to notify on Tailscale auth key expiry (guarded by a file). Also adds a top-level config flag Changes
Sequence Diagram(s)sequenceDiagram
participant Tailscale as Tailscale Service
participant Script as post-tailscaled/run
participant API as api.core.sh
participant HA as Home Assistant Supervisor API
participant UI as Persistent Notification Service
Tailscale->>Script: start / provide key expiry info
Script->>Script: check expiry & /data/key_expiry_notification_is_done
alt key expired AND no guard file
Script->>API: api.core POST services/persistent_notification/create (JSON)
API->>HA: HTTP request with Authorization
HA-->>API: 200 OK / response
API-->>Script: success
Script->>Script: create /data/key_expiry_notification_is_done
HA->>UI: display persistent notification
else guard exists OR not expired
Script->>Script: skip notification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tailscale/rootfs/usr/lib/core.sh:
- Around line 4-11: The function log.error_or_warning incorrectly forwards
arguments using unquoted $* which breaks when arguments contain whitespace;
change both bashio::log.error $* and bashio::log.warning $* to forward the
original arguments as "$@" so argument boundaries and spaces are preserved when
calling bashio::log.error and bashio::log.warning from log.error_or_warning.
- Around line 38-49: The local variable declarations are syntactically wrong and
declare an unused variable; in the scope where getopts is used (the while
getopts "sw" o; do ... done block in core.sh) replace "local OPTIND o a" with
"local OPTIND o" (remove unused 'a'), and replace "local silent= local
warning_only=" with proper initializations like "local silent= warning_only=" or
"local silent=0 warning_only=0" depending on expected truthiness; also add a
default *) case inside the case statement to handle invalid flags (e.g., print
an error or return/non-zero) so the getopts loop robustly handles unexpected
options.
🧹 Nitpick comments (1)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (1)
167-178: Consider improving error handling for notification creation.The notification logic correctly prevents duplicates, but has potential error handling concerns:
Line 176: The
-wflag (warning only) combined with> /dev/nullsuppresses all output and downgrades errors to warnings. If the API call fails, users may not notice, though the log warnings still provide visibility.Line 177: The marker file is created regardless of whether the API call succeeded. If the notification creation fails, the marker prevents future retry attempts.
Consider checking the exit status of
core.apibefore creating the marker file to allow retry on failure.♻️ Proposed improvement for error handling
# Create persistent notification also, but only once if ! bashio::fs.file_exists "/data/key_expiry_notification_is_done"; then attributes=$(bashio::var.json \ title "Tailscale Add-on" \ message "$(printf "%s\n\n" \ "**The connection's key will expire on: ${keyexpiry}!**" \ "Consider disabling key expiry to avoid losing connection to your Home Assistant device." \ "You can use the [Machines page](https://login.tailscale.com/admin/machines) of Tailscale's admin console to disable key expiry." \ "Please check your configuration based on the add-on's documentation under the \"Configuration\" section.")") - core.api POST services/persistent_notification/create "${attributes}" -w > /dev/null - touch "/data/key_expiry_notification_is_done" + if core.api POST services/persistent_notification/create "${attributes}" -w > /dev/null; then + touch "/data/key_expiry_notification_is_done" + fi fiThis ensures the marker file is only created when the notification is successfully created.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tailscale/config.yamltailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/usr/lib/core.sh
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
📚 Learning: 2025-11-06T10:55:55.062Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
Applied to files:
tailscale/rootfs/usr/lib/core.shtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 419
File: tailscale/rootfs/command/with-contenv-merge:8-12
Timestamp: 2024-11-15T17:38:56.576Z
Learning: The script `with-contenv-merge` in `tailscale/rootfs/command/` is directly copied from s6 and should not be modified.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 Learning: 2025-11-23T00:06:06.013Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/config.yaml
📚 Learning: 2024-10-09T10:32:14.110Z
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/config.yaml
📚 Learning: 2025-09-17T23:53:19.467Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 509
File: tailscale/config.yaml:0-0
Timestamp: 2025-09-17T23:53:19.467Z
Learning: In the Tailscale add-on config.yaml, there is an intentional naming mismatch between map entries and schema field names: map entries use actual Home Assistant directory names (like `homeassistant_config`, `all_addon_configs`) while schema fields use user-friendly names (like `config`, `addon_configs`). This allows correct directory mapping while presenting cleaner option names in the UI.
Applied to files:
tailscale/config.yaml
🪛 Shellcheck (0.11.0)
tailscale/rootfs/usr/lib/core.sh
[warning] 7-7: Use "$@" (with quotes) to prevent whitespace problems.
(SC2048)
[warning] 9-9: Use "$@" (with quotes) to prevent whitespace problems.
(SC2048)
[warning] 38-38: a appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 39-39: Remove space after = if trying to assign a value (for empty string, use var='' ... ).
(SC1007)
[error] 39-39: This applies local to the variable named local, which is probably not what you want. Use a separate command or the appropriate declare options instead.
(SC2316)
[warning] 39-39: local appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 41-48: Invalid flags are not handled. Add a *) case.
(SC2220)
🔇 Additional comments (9)
tailscale/config.yaml (1)
17-17: LGTM! Configuration change enables Home Assistant API access.The addition of
homeassistant_api: truecorrectly enables API access needed for the persistent notification feature.tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (3)
8-8: LGTM! Sourcing the new utility library.The script correctly sources the new core library that provides the API wrapper functions.
18-18: LGTM! Variable declaration for notification attributes.The declaration of
attributesis appropriate for storing the JSON payload used in the notification logic.
161-165: LGTM! Date formatting and improved warning messages.The key expiry date formatting and updated warning text correctly guide users to the configuration documentation.
tailscale/rootfs/usr/lib/core.sh (5)
26-36: LGTM! Function signature and argument parsing logic.The function correctly handles method, resource, optional POST data, and optional jq filter arguments.
51-71: LGTM! Authorization and HTTP request logic.The function correctly handles authorization headers and executes the curl request with appropriate error handling.
73-80: LGTM! Response parsing and debug logging.The code correctly extracts the HTTP status code and response body, with appropriate debug logging.
82-112: LGTM! HTTP status code handling.The function provides comprehensive error handling for common HTTP status codes (400, 401, 403, 404, 405) and handles unknown errors appropriately.
114-121: LGTM! Optional jq filtering and response output.The function correctly applies optional jq filtering and returns the response with appropriate exit codes.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @tailscale/rootfs/usr/lib/core.sh:
- Around line 117-124: If bashio::jq fails when applying the filter, do not echo
partial/empty output or return __BASHIO_EXIT_OK; instead capture the exit status
of bashio::jq after calling it with response and filter, and on non-zero status
call bashio::log.error with a descriptive message (including the filter and any
stderr if available) and return __BASHIO_EXIT_NOK; only echo "${response}" and
return __BASHIO_EXIT_OK when bashio::jq succeeds or when no filter was provided.
- Around line 76-115: Strip any trailing newlines from the extracted status
reliably and guard all numeric comparisons by first validating status is
numeric: after extracting status, remove newlines (e.g.,
status=${status//$'\n'/} and response=${response%$'\n'}), then check with a
regex like [[ "${status}" =~ ^[0-9]+$ ]] and if it fails call
log.error_or_warning "${warning_only}" "Unexpected non-numeric HTTP status:
${status}" and return "${__BASHIO_EXIT_NOK}"; only perform the existing [[
"${status}" -eq ... ]] checks when the numeric validation passes.
- Around line 26-53: The core.api function misuses getopts without resetting
OPTIND and does blind shifts that can fail if callers pass too few args; before
any shifts validate that required arguments (method and resource) exist using
parameter checks like "${1:-}" and return or error early if missing, then safely
assign method and resource, and before invoking getopts reset OPTIND=1 so option
parsing is deterministic; ensure you only shift when you confirmed an argument
exists (use guarded shifts) and keep option variables (silent, warning_only) as
before.
- Around line 54-84: The function currently always sends -d "${data}" and
doesn't capture curl stderr or set timeouts and logs raw request bodies; update
the curl invocation in the API call to: omit the -d option when "${method}" is
GET (only include -d when method != GET), redirect stderr into the response
capture (append 2>&1 to the subshell command) and add connection/read timeouts
(e.g. --connect-timeout and --max-time flags), and remove or redact logging of
the raw request body (avoid bashio::log.debug "Request data: ${data}" or only
log a masked/summary value when method != GET) while keeping existing variables
(method, data, response, status, __BASHIO_SUPERVISOR_TOKEN,
__BASHIO_SUPERVISOR_API) and same error path (log.error_or_warning and return
__BASHIO_EXIT_NOK).
🧹 Nitpick comments (1)
tailscale/rootfs/usr/lib/core.sh (1)
4-11: Makewarning_onlya boolean flag (don’t overload “has value”).
Right now any non-empty first arg flips to warning, which is easy to misuse (e.g., passing a message accidentally as first arg). Consider explicit0/1semantics and clearer call sites.Proposed tweak
function log.error_or_warning() { - local warning_only="$1"; shift - if ! bashio::var.has_value "${warning_only}"; then + local warning_only="${1:-0}"; shift || true + if [[ "${warning_only}" == "0" ]]; then bashio::log.error "$@" else bashio::log.warning "$@" fi }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/rootfs/usr/lib/core.sh
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
📚 Learning: 2025-10-29T20:47:02.818Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 562
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run:120-128
Timestamp: 2025-10-29T20:47:02.818Z
Learning: In the hassio-addons/addon-tailscale project, when checking for IPv6 addresses in bash scripts, use the regex format `if [[ "${variable}" =~ .*:.* ]]; then` for consistency with other IPv6 tests in the codebase.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-11-06T10:55:55.062Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-09-16T23:47:20.987Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:30-37
Timestamp: 2025-09-16T23:47:20.987Z
Learning: In the hassio-addons/addon-tailscale project, the advertise_routes configuration uses `local_subnets` (with underscore) as the default value, and the runtime regex patterns use `local.subnets` (with dot) which correctly matches the underscore variant because '.' in regex matches any single character including '_'.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 419
File: tailscale/rootfs/command/with-contenv-merge:8-12
Timestamp: 2024-11-15T17:38:56.576Z
Learning: The script `with-contenv-merge` in `tailscale/rootfs/command/` is directly copied from s6 and should not be modified.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run:
- Around line 167-179: The current one-time guard using the file
/data/key_expiry_notification_is_done prevents re-notification when a new key
expiry appears; change the guard logic around the keyexpiry check so you store
the expiry value in the marker instead of just touching it: read the marker if
present and compare its contents to the current keyexpiry, and only call
core.api POST services/persistent_notification/create with attributes and then
update/write the marker with the current keyexpiry when they differ (replace the
touch "/data/key_expiry_notification_is_done" behavior with writing keyexpiry),
or alternatively remove the guard entirely if you want notifications on every
restart; update the logic surrounding attributes, keyexpiry and the marker file
accordingly.
- Around line 169-175: The notification currently hardcodes the admin URL;
update the attributes construction in the s6 run script to use the configured
login_server variable (e.g., use "${login_server%/}/admin/machines" or similar
to avoid double slashes) instead of
"https://login.tailscale.com/admin/machines", and ensure you fall back to
"https://login.tailscale.com" when login_server is empty or unset (login_server
is declared earlier around lines 78-82) so custom login servers produce a
correct admin console link.
🧹 Nitpick comments (2)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (2)
161-161: Add format validation for robust keyexpiry parsing.The substring extraction assumes ISO 8601 format (
YYYY-MM-DDTHH:MM:SSZ). If Tailscale changes theKeyExpiryformat in the future, this will silently produce malformed output.🛡️ Proposed validation and error handling
+# Validate and format keyexpiry +if [[ ! "${keyexpiry}" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2} ]]; then + bashio::log.warning "Unexpected KeyExpiry format: ${keyexpiry}" + keyexpiry_display="${keyexpiry}" +else - keyexpiry="${keyexpiry:0:10} ${keyexpiry:11:8} UTC" + keyexpiry_display="${keyexpiry:0:10} ${keyexpiry:11:8} UTC" +fi - bashio::log.warning "The connection's key will expire on: ${keyexpiry}" + bashio::log.warning "The connection's key will expire on: ${keyexpiry_display}"Alternatively, consider using
datefor more robust parsing:keyexpiry_display=$(date -d "${keyexpiry}" "+%Y-%m-%d %H:%M:%S UTC" 2>/dev/null) || keyexpiry_display="${keyexpiry}"
167-168: Consider logging when notification is skipped.When the guard file exists, the notification creation is silently skipped. Adding a debug log message would help administrators understand why they're not seeing a notification on subsequent restarts.
📝 Suggested logging enhancement
# Create persistent notification also, but only once - if ! bashio::fs.file_exists "/data/key_expiry_notification_is_done"; then + if ! bashio::fs.file_exists "/data/key_expiry_notification_is_done"; then attributes=$(bashio::var.json \ title "Tailscale Add-on" \ message "$(printf "%s\n\n" \ "**The connection's key will expire on: ${keyexpiry}!**" \ "Consider disabling key expiry to avoid losing connection to your Home Assistant device." \ "You can use the [Machines page](https://login.tailscale.com/admin/machines) of Tailscale's admin console to disable key expiry." \ "Please check your configuration based on the add-on's documentation under the \"Configuration\" section.")") if core.api POST services/persistent_notification/create "${attributes}" -w > /dev/null; then touch "/data/key_expiry_notification_is_done" fi + else + bashio::log.debug "Key expiry notification already sent; skipping (guard file exists)" fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 419
File: tailscale/rootfs/command/with-contenv-merge:8-12
Timestamp: 2024-11-15T17:38:56.576Z
Learning: The script `with-contenv-merge` in `tailscale/rootfs/command/` is directly copied from s6 and should not be modified.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 Learning: 2025-11-06T10:55:55.062Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 Learning: 2025-11-23T00:06:06.013Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 Learning: 2024-10-09T10:32:14.110Z
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tailscale/rootfs/usr/lib/core.sh:
- Around line 41-52: The getopts loop can fail on subsequent calls because
OPTIND is not reset; before the while getopts "sw" o; do block (which sets
silent and warning_only), reset OPTIND (e.g. OPTIND=1 or declare a local OPTIND)
so option parsing starts fresh each call, ensuring getopts processes options
correctly across multiple invocations.
🧹 Nitpick comments (2)
tailscale/rootfs/usr/lib/core.sh (2)
34-34: Consider using-nfor improved readability.The condition
! -zworks correctly but-nis more idiomatic for checking if a variable has a value.♻️ Suggested refactor
- if [[ ! -z "${1:-}" && "${1::1}" != "-" ]]; then + if [[ -n "${1:-}" && "${1::1}" != "-" ]]; then
54-74: Consider adding validation for missing authentication token.If
__BASHIO_SUPERVISOR_TOKENis not set, the Authorization header will beAuthorization: Bearerwithout a token value (Line 54), which will likely result in a 401 authentication error. While this error would be handled by the status code checks later, it might be more defensive to detect and log this condition upfront.Consider adding a check after Line 62:
if [[ "${auth_header}" == "Authorization: Bearer" ]]; then bashio::log.warning "Supervisor token not available, API call may fail authentication" fiThis would make it clearer when authentication credentials are missing, rather than relying on interpreting HTTP 401 responses.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/rootfs/usr/lib/core.sh
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 605
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run:167-179
Timestamp: 2026-01-10T23:19:59.890Z
Learning: In the Tailscale add-on, the key expiry notification in `tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run` is intentionally designed to be shown only once per device lifecycle using the guard file `/data/key_expiry_notification_is_done`. This one-time behavior is the desired design.
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
📚 Learning: 2026-01-10T23:18:22.775Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 605
File: tailscale/rootfs/usr/lib/core.sh:26-53
Timestamp: 2026-01-10T23:18:22.775Z
Learning: In tailscale/rootfs/usr/lib/core.sh, the core.api() function uses a design pattern where options (-s, -w) are placed AFTER all positional arguments (method, resource, optional POST data, optional jq filter), as documented in the function's comment block.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-10-29T20:47:02.818Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 562
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run:120-128
Timestamp: 2025-10-29T20:47:02.818Z
Learning: In the hassio-addons/addon-tailscale project, when checking for IPv6 addresses in bash scripts, use the regex format `if [[ "${variable}" =~ .*:.* ]]; then` for consistency with other IPv6 tests in the codebase.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-11-06T10:55:55.062Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-09-16T23:47:20.987Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:30-37
Timestamp: 2025-09-16T23:47:20.987Z
Learning: In the hassio-addons/addon-tailscale project, the advertise_routes configuration uses `local_subnets` (with underscore) as the default value, and the runtime regex patterns use `local.subnets` (with dot) which correctly matches the underscore variant because '.' in regex matches any single character including '_'.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 419
File: tailscale/rootfs/command/with-contenv-merge:8-12
Timestamp: 2024-11-15T17:38:56.576Z
Learning: The script `with-contenv-merge` in `tailscale/rootfs/command/` is directly copied from s6 and should not be modified.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-12-26T14:17:39.622Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 455
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-magicdns-proxies/run:73-75
Timestamp: 2025-12-26T14:17:39.622Z
Learning: In the hassio-addons projects using bashio, the bashio library automatically sets `set -o errexit`, `set -o errtrace`, `set -o nounset`, and `set -o pipefail` by default (see https://github.com/hassio-addons/bashio/blob/main/lib/bashio lines 10-13). Scripts using `#!/command/with-contenv bashio` or `#!/usr/bin/env bashio` inherit this behavior, so explicit error handling is not required for commands that exit with non-zero status codes.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-09-16T23:47:20.987Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:30-37
Timestamp: 2025-09-16T23:47:20.987Z
Learning: In regex patterns, '.' matches any single character, so `local.subnets` in a regex would match `local_subnets`, `localXsubnets`, etc. The user lmagyar correctly pointed this out when I mistakenly identified it as a literal string mismatch.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2025-09-17T21:52:47.107Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh:68-72
Timestamp: 2025-09-17T21:52:47.107Z
Learning: I made a critical regex error by claiming that the pattern "local.subnets" wouldn't match "local_subnets". In regex, '.' matches any single character including '_', so this pattern works correctly. The user lmagyar had to correct this same basic regex misunderstanding twice.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
📚 Learning: 2026-01-10T23:19:59.890Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 605
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run:167-179
Timestamp: 2026-01-10T23:19:59.890Z
Learning: In the Tailscale add-on, the key expiry notification in `tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run` is intentionally designed to be shown only once per device lifecycle using the guard file `/data/key_expiry_notification_is_done`. This one-time behavior is the desired design.
Applied to files:
tailscale/rootfs/usr/lib/core.sh
🔇 Additional comments (3)
tailscale/rootfs/usr/lib/core.sh (3)
1-3: LGTM!The shebang and shellcheck directive are appropriate for a bash library file that will be sourced by other scripts.
4-11: LGTM!The function correctly handles conditional error/warning logging based on the first parameter value.
76-124: LGTM!The response parsing, HTTP status code handling, and error reporting are well-structured. The function correctly:
- Extracts the HTTP status code from curl output
- Handles all common HTTP error codes with appropriate messages
- Respects the
-s(silent) and-w(warning-only) flags- Applies optional jq filtering when specified
- Returns appropriate exit codes
lmagyar
force-pushed
the
pr-key-expiry-notification
branch
from
45b3280 to
2019d1b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tailscale/rootfs/usr/lib/api.core.sh`:
- Around line 26-36: The POST argument shifting currently unconditionally
consumes the next positional arg; update the api.core function so data is only
consumed and shifted when a POST payload is actually provided: check the next
argument (after resource) for existence and that its first character is not "-"
(e.g., [[ -n "${1:-}" && "${1::1}" != "-" ]]) before assigning data="${1}" and
doing the shift; keep data='{}' otherwise so subsequent option parsing
(filter/getopts) sees flags like -w intact. Ensure this logic references the
existing locals method, resource, data, and filter in api.core.
frenck
requested changes
Feb 5, 2026
frenck
left a comment
frenck
left a comment
Member
There was a problem hiding this comment.
Persistent notifications are user space. This shouldn't be used by integrations (and also thus not add-ons).
../Frenck
Blogging my personal ramblings at frenck.dev
lmagyar
force-pushed
the
pr-key-expiry-notification
branch
from
5ab62c4 to
5764317
Compare
Collaborator
Author
|
Integrations can use the repair/issue system, but that is not available for an add-on/app. Persistent notifications are the only way to create a message for the user about an important situation that can cause losing access to the device. Like we get failed login attempt notifications from the http/auth integrations. I think it is useful, but it's your decision, I can live with it. |
Member
That doesn't make this PR a good thing. Persistent notification are a forbidden area in Home Assistant in that regard. Additionally, these are also visible for secondary users, who can absolutely do nothing with it. I rather explore the ability for apps to raise repairs (which I really think it discussable) |
Collaborator
Author
|
OK, I mark it as draft, until a SU API is available. Note: Issue fields that are hard to set for an add-on:
|
…xpiration is detected
lmagyar
force-pushed
the
pr-key-expiry-notification
branch
from
5764317 to
f5ce9bc
Compare
Closed
lmagyar
added
new-feature
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
2 participants
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.
Proposed Changes
If we thought that this is the only warning we should highlight in the docs, it worths a notification.
But this will be created only once after install.
Note to myself: notification is also useful for invalid networking DNS config check, see: lmagyar@6669e24
Related Issues
fixes #603
Summary by CodeRabbit
New Features
Improvements