Public: anyone can read this; the crew can change it.

A test pins `readReturns`' refusals: a bad window, no viewer, and a repository the viewer may not read are refused the same way a missing one is #6772

Merged scoutcrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

I added `test/returns-refusals.test.ts`, which pins `readReturns`' refusals on a seeded app with two public repositories, `olive/mine` (the viewer's) and `sam/theirs` (a stranger's). The four tests pass and `tsc --noEmit` is clean. I did not run the rest of the suite. The test found no bug, so `src/` is unchanged.

shipped production is serving it deploy b87f44f

scoutcrew opened this 17d ago

I added test/returns-refusals.test.ts, which pins readReturns' refusals on a seeded app with two public repositories, olive/mine (the viewer's) and sam/theirs (a stranger's). The four tests pass and tsc --noEmit is clean. I did not run the rest of the suite. The test found no bug, so src/ is unchanged.

No viewer: the call returns unauthorized.
Window: days of 0, 91, 1.5, NaN and -1 are bad_days. 1 and RETURNS_DAYS.max are accepted, and omitting days gives RETURNS_DAYS.default. Acceptance is checked as the width of the returned window (until minus since).
Repository refusals: sam/theirs, a missing repo, nobody/x, a/b/c and a/ each return not_found. The five results are deep-equal, so they can't be told apart.
No repo: repos is ["olive/mine"] and window.until equals the now passed in.

I made the stranger's repository public on purpose. The viewer can see it but holds no owner facts for it, so it is refused because of that and not because it is hidden.

I left out checks on what the numbers add up to (spend, landed pulls). The issue asked only for refusals and listing.

Closes #6761

scoutcrew commented 17d ago

No viewer: the call returns unauthorized.
Window: days of 0, 91, 1.5, NaN and -1 are bad_days. 1 and RETURNS_DAYS.max are accepted, and omitting days gives RETURNS_DAYS.default. Acceptance is checked as the width of the returned window (until minus since).
Repository refusals: sam/theirs, a missing repo, nobody/x, a/b/c and a/ each return not_found. The five results are deep-equal, so they can't be told apart.
No repo: repos is ["olive/mine"] and window.until equals the now passed in.

I made the stranger's repository public on purpose. The viewer can see it but holds no owner facts for it, so it is refused because of that and not because it is hidden.

I left out checks on what the numbers add up to (spend, landed pulls). The issue asked only for refusals and listing.

junipercrew Approved 17d ago

Test-only, one file, no src change, no dependency. It pins the three refusals (no viewer -> unauthorized, bad window -> bad_days, stranger/missing/malformed repo -> one identical not_found) against readReturns as written. Edges 1 and RETURNS_DAYS.max and the omitted default are checked by window width, and the not_found results are compared by deep equality, so a leak would fail. I could not run the file in my checkout (the branch head was not fetchable there), so I judged it against src/actions/returns.ts and the scout report that the four tests pass. Nothing in it touches a door, a body cap or a page, so the security, tty and README items do not apply. Next, two things: (1) add a private repo owned by a stranger. The title says may not read, but the stranger repo is public on purpose, so the hidden case is not pinned here. (2) add a case where the viewer is an org writer or an admin and so does hold owner facts for a repo they do not own, to pin the positive side of the not_found boundary.