Skip to content

fix: an artifact whose expiresAt cannot be read now serves 410 (T1.2.22) - #45

Merged
kuyazee merged 3 commits into
mainfrom
task/artifact-expiry-junk
Aug 12, 2026
Merged

fix: an artifact whose expiresAt cannot be read now serves 410 (T1.2.22)#45
kuyazee merged 3 commits into
mainfrom
task/artifact-expiry-junk

Conversation

@kuyazee

@kuyazee kuyazee commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

The item

T1.2.22, backlog 1 epic 1.2, filed 2026-08-11 from the T1.2.20 adversarial and security lenses.

isExpired read Boolean(meta.expiresAt && Date.parse(meta.expiresAt) <= Date.now()). Date.parse
answers NaN for junk and NaN <= now is false, so an artifact whose stored expiresAt cannot be
read kept serving instead of answering 410. Five gates use it, including the capability-link
exchange, so the auto-expire lifecycle README.md:52 promises was off for any record carrying such a
value. It is the artifact twin of the key bug T1.2.20 fixed.

parseExpiresAt refuses anything but an ISO string on the way in, so the reachable surface is a
hand-edited or restored meta.json, the same surface T1.2.11 and T1.2.20 covered.

What changed

  • lib/expiry.js is new and holds artifactExpired. Absent, null and '' still mean no expiry.
    A non-string is lapsed without asking Date.parse, because Date.parse stringifies first and
    then reads more than an operator would expect: 12345 becomes the year 12345 and
    ["2030-01-01"] becomes the string inside the array. Anything else that does not parse is lapsed.
    It lives outside server.js so a unit test can hand it the shapes an API call cannot make.
  • server.js keeps its short local name (const isExpired = artifactExpired) so the five call
    sites read as before.
  • public/index.html carried a hand-copied second version of the rule for the row badge. It now
    follows the same one, with a comment on both sides saying a change belongs in both places.
  • cli.js prints expired for a lapsed artifact. It printed the stored value and never said
    whether it had passed, so a dead artifact listed like a live one and a stored object printed as
    expires [object Object].
  • docs/api.md says the 410 covers a value that cannot be read at all, not only a date in the past.

Review

Four lenses, because the diff touches the serve paths. Twelve findings, six fixed here, three filed,
three recorded.

Fixed here, and each reproduced first:

  1. A non-string that stringifies to a readable date stayed immortal. Adversarial and security
    and QA, independently. artifactExpired({expiresAt: 2030}) was false, and a server booted from
    the first commit served /a/junkpub 200 with meta.expiresAt = 2030 and again with
    ["2030-01-01T00:00:00Z"]. Closed by the non-string rule above.
  2. The row menu died on a non-string expiresAt, on exactly the row the new badge points at.
    Adversarial, security, QA and UX, all four. a.expiresAt.slice(0, 10) threw inside the lazy
    fill, so the menu stopped after QR code: no Expiry, no Disable, no Delete. It never recovered
    either, because fill only runs while the menu has no children, so reopening showed the same
    truncated list for the rest of the session. Coerced once before the label and the dialog value.
  3. POST /a/:slug/unlock answered 410 before checking visibility. Security. That route's 404
    and 401 are deliberately uniform so it never distinguishes an artifact's mode, and the two GET
    paths already order expiry after that rule. The unlock route did not, so an anonymous caller
    learned that a private slug exists. The check now runs after the password is proven. Measured on
    both trees: on main, unlock with the right password on an expired artifact answered 200 and
    served it; here it answers 410, a wrong password still answers 401, and a missing slug still
    answers 404.
  4. The test could not catch a regression to the shorter guard. QA. Every value in the first
    junk list was NaN under Date.parse, so swapping the rule for if (!value) return false kept
    the suite green while false and 0 went back to meaning no expiry. Both are in the list now,
    with 12345 and a one-element array.
  5. Nothing drove the fix end to end. QA. The unit test is one level shallower than the five call
    sites. ci.yml now plants "garbage" into meta.json between two boots, the way the
    auth.json step from T1.2.19 does, and checks 410 on /a/:slug and /a/:slug/source, then
    checks that a PATCH {"expiresAt": null} brings the artifact back.
  6. docs/api.md understated when a 410 fires. QA and UX and adversarial.

Filed, because each is bigger than this diff or needs a call from Z:

  • T1.2.24: an artifact broken by its stored expiry renders the same as one that simply lapsed.
    The key side already has broken plus copy that names the reason; the artifact side has no such
    field. About 20 to 30 lines over four files.
  • T1.2.25: a lapsed artifact serves a bare text 410 while every other miss gets the designed
    not-found shell. Pre-existing, and this change puts more artifacts into that state. Needs Z on
    whether the 410 gets its own copy.
  • T1.2.26: Copy on an expired row mints a capability link and flashes Copied, and the link
    404s. GET /api/artifacts/:slug/link is not one of the five gates.

Recorded, no action:

  • Typing an expiry the server will reject now greys the row for the length of one request.
    patchItem applies the typed string locally before the fetch, and under the new rule a value like
    tomorrow scores as lapsed. It reverts on the 400 with a toast. Adversarial, low.
  • POST /a/:slug/unlock still separates a missing slug (404) from an existing private one (401),
    and the 404 branch returns before the rate limiter records a failure. Pre-existing, unrelated to
    expiry, and larger than this diff. Security lens flagged it as worth its own item.
  • V8 and JSC disagree on nothing here. The security lens compared the server rule and the browser
    rule across 16 shapes, including "0", "2026-13-45", "2026/08/12" and [1], and found no
    case where the badge and the response part ways.

Tests

  • npm test: 49 to 52. Three new cases in test/expiry.test.js covering eleven junk shapes, the
    three absent shapes, and a past and a future date.

  • bash .github/workflows/smoke.sh http://localhost:3411 test: 154 ok-lines, all pass, same count
    as origin/main.

  • End to end by hand on two local servers, with the record planted in meta.json between two boots:

    origin/main this branch
    GET /a/junkpub 200 410
    GET /a/junkpub/source 200 410
    GET /a/junkpw (password artifact) 401 404
    unlock with the wrong password 401 401
    unlock with the right password 200, and it served 410
    unlock on a slug that does not exist 404 404
    PATCH {"expiresAt": null} then GET 200 200
  • Browser at 1280x900, four artifacts, three with a planted expiry. All three carry the expired
    pill, the healthy one does not, the {} row's menu reaches Expiry, Disable and Delete, and the
    page logs 0 console errors and 0 warnings.

  • artifacts list against the same instance prints expired on all three and nothing on the
    healthy one.

Screenshot: https://artifacts.zonily.cloud/a/w3n4hmvyxi

Merging

Checked against both open PRs with git merge-tree. This branch merges clean with #43 and with #44,
in either order.

isExpired read NaN as "not expired", so junk in a hand-edited or restored meta.json kept serving instead of answering 410. Five call sites gate on it, including the capability-link exchange, so the auto-expire lifecycle README.md:52 promises was off for any record carrying an unreadable value.

The rule moves to lib/expiry.js as artifactExpired, the artifact twin of keyExpired: absent, null and empty still mean no expiry, anything present that Date.parse cannot read counts as lapsed. It lives outside server.js because parseExpiresAt refuses these values on the way in, so no request can set one up and no end-to-end test can reach the case.

The dashboard row carried the same read and now follows the same rule.
Four lenses on the first cut. What changed here:

- artifactExpired refuses a non-string before asking Date.parse. Date.parse stringifies first, so 12345 read as the year 12345 and ["2030-01-01"] read as the string inside the array, and both kept serving forever. The dashboard rule follows.
- The row menu no longer dies on a non-string expiresAt. slice() threw inside the lazy fill, which stopped the menu at QR code: no Expiry, no Disable, no Delete, and no retry on reopen, so the row the new badge points at was the one the operator could not fix.
- POST /a/:slug/unlock checks expiry after the password rather than before it. The 404/401 pair over that route is deliberately uniform, and a 410 handed out first told an anonymous caller that a private slug exists.
- artifacts list prints expired instead of the stored value, so a dead artifact stops listing like a live one and "expires [object Object]" is gone.
- ci.yml plants an unreadable expiresAt in meta.json between two boots and drives the real serve paths, which is the only way to reach the case end to end.
- docs/api.md says 410 covers a value that cannot be read at all, and the test carries the false and 0 cases that separate this from the shorter falsy check.
DEFAULT_VISIBILITY is private, so the fixture published private and /a/expiry-junk answered 404 before the step ever reached the interesting part. A private artifact 404s for both an expired and a live record, so the case has to run on a public one.
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