Conversation
This parses the timestamps the project stores for secret expiry, in the format
"%Y-%m-%dT%H:%M:%SZ" — no fractional seconds. That exact format was broken on both
platforms, in two independent ways.
1. Z duplication — broke GNU/Linux (Debian, Ubuntu).
`${ts%%.*}` is a no-op when there is no fractional part, so the trailing Z survived
and a second one was appended: "2026-03-03T10:00:00ZZ". GNU date rejects that, and
`date -D` does not exist on GNU, so the function fell through to `echo 0`. Callers
read 0 as "no expiry":
- secret_check_expiry skips the entry, so an expired secret is never disabled
- secret_extend treats it as already expired and extends from now, silently
granting more days than were asked for
Measured: on Debian 12 and Ubuntu 24.04 the stored format returned 0 while the same
value with fractional seconds returned correctly — which is why this went unnoticed.
2. Local-time parsing — broke Alpine/busybox.
The busybox branch strips the Z and then parses with no TZ. Under `-D`, busybox
ignores a trailing Z entirely, so a UTC value was reinterpreted as local time. The
error equals the offset that timestamp's own date carries, so on a DST zone it is an
hour in winter and two in summer.
Both copies are fixed: the manager's, and the comment-stripped one inside the generated
daemon heredoc.
tests/test_iso_to_epoch.sh covers both — part A forces the busybox branch through a date
stub, part B uses the real date — so each defect is caught on either platform. It fails
4/9 on Debian 12 and 5/9 on Alpine 3.20 before the change, and passes 9/9 on Debian,
Ubuntu, Alpine and Fedora after.
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.
Summary
_iso_to_epoch()parses the ISO 8601 timestamps this project stores for secret expiry, in the format"%Y-%m-%dT%H:%M:%SZ"— no fractional seconds. That exact format was broken on both platforms, by two independent defects. Values with fractional seconds happened to work, which is why neither was noticed.Defect 1 — Z duplication (GNU/Linux: Debian, Ubuntu)
${ts%%.*}is a no-op when there is no fractional part, so the trailingZsurvived and a second one was appended:
GNU
daterejects that, anddate -Ddoes not exist on GNU, so the function fellthrough to
echo 0.Measured on Debian 12 and Ubuntu 24.04:
Impact
Callers read
0as "no expiry":secret_check_expiry(:3004) —[ "$exp_epoch" -le 0 ] && continue— skips the entry, so an expired secret is never disabledsecret_extend(:2834) —[ "$base_epoch" -le "$now_epoch" ] && base_epoch=$now_epoch— treats it as already expired and extends from now, silently granting more days than requestedDefect 2 — local-time parsing (Alpine/busybox)
The busybox branch strips the
Zand then parses without forcing a timezone:bash local ts_bb="${ts_clean%Z}"epoch=$(date -D '%Y-%m-%dT%H:%M:%S' -d "${ts_bb}" +%s ...) # no TZUnder
-D, busybox ignores a trailingZentirely, so a UTC value is reinterpreted as local time:Both copies are fixed: the manager's, and the comment-stripped copy inside the generated daemon's heredoc (
TELEGRAM_SCRIPT), which the daemon uses for its ownStartedAtage calculation.Testing
New
tests/test_iso_to_epoch.sh. It has to cover both defects on both platforms, so it runs in two parts:datestub onPATHmodelling busybox's real option surface — plain-d <value ending in Z>rejected,-D FMT -d <value>accepted with theZignored — and runs underTZ=XXX-2(a POSIX zone string, so no tzdata is needed). This forces the busybox branch on GNU hosts.date, which is where the doubledZsurfaces on GNU.Neither part alone would catch both. Before the change it fails 4/9 on Debian 12 and 5/9 on Alpine 3.20; after, it passes 9/9 on Debian 12, Ubuntu 24.04, Alpine 3.20 and Fedora 41, with no other test result changing.