Problem
The tzdata bundling that fixed #2647 shipped two defects. Both were introduced by that PR's final commit (e86c2a98, "ship tzdata as zip to satisfy typos"), which changed vendoring from a checked-in directory tree to a runtime-extracted zip — after the last security review. Both bot reviews pin Last security-reviewed head: 1b514176, which used a plugin-local directory and never extracted anything.
The #2647 fix itself is good and is not in question: IANA zones now resolve on a Windows host with no system TZDB.
1. A failing bundle reports "the limit still holds"
_ensure_bundled_tzdata() has no exception handling around ZipFile / extractall. A corrupt or truncated vendor zip raises at import time and the script exits 1.
Exit 1 in this script's contract means the limit is still active — hand back and stop. So a damaged bundle does not degrade to "cannot determine"; it actively tells the caller to keep waiting. A missing bundle already degrades correctly to exit 3, which makes the inconsistency plain: the one failure mode that was handled is the one least likely to happen.
The same uncaught class covers a read-only TEMP, disk-full during extraction, an unreadable bundle, and the staging.rename(cache_root) re-raise.
2. A predictable shared path feeds sys.path.insert(0, …)
The cache directory is <gettempdir()>/session-flow-tzdata-<sha256(zip)[:16]>. The digest derives from a publicly committed file, so the path is fully predictable. The cache-hit branch (if not (cache_root / "tzdata").is_dir()) skips extraction entirely, puts the directory at sys.path[0], and import tzdata runs whatever is there.
Evidence
Reproduced on Windows 11, CPython 3.14, against origin/main.
Defect 1 — scratch copy of the script with a sabotaged bundle, asking a question whose answer should be "lifted":
corrupt zip exit=1 (expected 3)
truncated zip exit=1 (expected 3)
bundle path is a directory exit=3
missing zip exit=3
Defect 2 — computing the cache path from the committed zip's digest, planting tzdata/__init__.py there, then running the script:
planted=session-flow-tzdata-40438214aeac6481 executed=True
Severity, stated in both directions: on Windows %TEMP% is per-user, so this is not a cross-user vector on the platform where #2647 was reported. On Linux or macOS with a shared world-writable /tmp and no TMPDIR, any local user can pre-plant against a fully predictable path — CWE-377, CWE-426.
Proposed change
- Wrap the body of
_ensure_bundled_tzdata() so every failure returns quietly. The caller then degrades to exit 3, exactly as a missing bundle already does.
- Add a per-user component to the cache directory name, and trust a cache hit only when the directory is not a symlink and — where the platform can report ownership — is owned by the current user.
os.getuid does not exist on Windows, so the ownership half is a no-op there and the per-user name carries the isolation instead; that asymmetry should be documented rather than papered over.
Acceptance criteria
- A corrupt, truncated, or otherwise unusable bundle yields exit 3, never exit 1.
- Exit codes 0, 1, and 2 are unchanged for valid input.
- A cache directory planted at the pre-change predictable path is not imported.
- A symlinked cache root is not trusted.
- Regression tests cover the corrupt and truncated cases and fail against the pre-change code.
Related note, not fixed here
A typo'd zone in a real message (Not/AZone) now yields exit 3 with guidance to "report the timezone failure", where it previously yielded 2. Exit 3 is documented in keep-going/SKILL.md and the CHANGELOG so the contract change is disclosed, but genuinely unparsable input arguably still belongs at 2. Flagging for a maintainer decision rather than changing it.
Separately: the checker cannot parse a date-form reset clause. "resets Aug 18, 8pm (America/New_York)" returns unparsed: no reset clause found (exit 2), because only same-day time forms are recognized. A weekly limit states its reset that way, so the script is unparsable for exactly the longest limits — worth its own issue.
Problem
The tzdata bundling that fixed #2647 shipped two defects. Both were introduced by that PR's final commit (
e86c2a98, "ship tzdata as zip to satisfy typos"), which changed vendoring from a checked-in directory tree to a runtime-extracted zip — after the last security review. Both bot reviews pinLast security-reviewed head: 1b514176, which used a plugin-local directory and never extracted anything.The #2647 fix itself is good and is not in question: IANA zones now resolve on a Windows host with no system TZDB.
1. A failing bundle reports "the limit still holds"
_ensure_bundled_tzdata()has no exception handling aroundZipFile/extractall. A corrupt or truncated vendor zip raises at import time and the script exits 1.Exit 1 in this script's contract means the limit is still active — hand back and stop. So a damaged bundle does not degrade to "cannot determine"; it actively tells the caller to keep waiting. A missing bundle already degrades correctly to exit 3, which makes the inconsistency plain: the one failure mode that was handled is the one least likely to happen.
The same uncaught class covers a read-only
TEMP, disk-full during extraction, an unreadable bundle, and thestaging.rename(cache_root)re-raise.2. A predictable shared path feeds
sys.path.insert(0, …)The cache directory is
<gettempdir()>/session-flow-tzdata-<sha256(zip)[:16]>. The digest derives from a publicly committed file, so the path is fully predictable. The cache-hit branch (if not (cache_root / "tzdata").is_dir()) skips extraction entirely, puts the directory atsys.path[0], andimport tzdataruns whatever is there.Evidence
Reproduced on Windows 11, CPython 3.14, against
origin/main.Defect 1 — scratch copy of the script with a sabotaged bundle, asking a question whose answer should be "lifted":
Defect 2 — computing the cache path from the committed zip's digest, planting
tzdata/__init__.pythere, then running the script:Severity, stated in both directions: on Windows
%TEMP%is per-user, so this is not a cross-user vector on the platform where #2647 was reported. On Linux or macOS with a shared world-writable/tmpand noTMPDIR, any local user can pre-plant against a fully predictable path — CWE-377, CWE-426.Proposed change
_ensure_bundled_tzdata()so every failure returns quietly. The caller then degrades to exit 3, exactly as a missing bundle already does.os.getuiddoes not exist on Windows, so the ownership half is a no-op there and the per-user name carries the isolation instead; that asymmetry should be documented rather than papered over.Acceptance criteria
Related note, not fixed here
A typo'd zone in a real message (
Not/AZone) now yields exit 3 with guidance to "report the timezone failure", where it previously yielded 2. Exit 3 is documented inkeep-going/SKILL.mdand the CHANGELOG so the contract change is disclosed, but genuinely unparsable input arguably still belongs at 2. Flagging for a maintainer decision rather than changing it.Separately: the checker cannot parse a date-form reset clause.
"resets Aug 18, 8pm (America/New_York)"returnsunparsed: no reset clause found(exit 2), because only same-day time forms are recognized. A weekly limit states its reset that way, so the script is unparsable for exactly the longest limits — worth its own issue.