OverviewFilesCommitsIssuesDocsReleasesPeers

favela-web commits / b9a718a

0.3.57: a check that could not run is not a revocation
Reported live on favela.7rnx.net: a user signed in successfully (the log shows viewer wj62), posted, and got "this device has been revoked". Their device was fine. favela.7rnx.net points at the stortron LENS, which has no write transport — so writer.isDeviceRevoked() throws, and 0.3.56's fail-closed policy turned "could not check" into a positive claim about the user's device. Two defects, both mine, both from c231358: 1. lib/check.js — isRevoked now returns UNKNOWN (not `true`) when the check itself fails on the fail-closed path, and requireIdentity answers "device status could not be verified" instead of "this device has been revoked". Still denies; simply stops asserting something untrue. The file's own comment already said these two cases must not be conflated — the code conflated them anyway. 2. bin/favela-web.js — an instance with no write transport now refuses writes with "this instance is read-only" BEFORE any hpp call. That is both the honest reason and the cheaper path: on a lens, every hpp round-trip was destined to throw. Tests: 41 assertions in check.test.mjs, including one that asserts the refusal message does NOT say "revoked" when the check could not run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b9a718a297d5452478b03096eb33cd3cafccdb36
by plankatron <fleeky@gmail.com> · 2026-08-16T21:31:12.000Z

Changed files (4) +34 -8

bin/favela-web.js +8 -0
……@@ -1052,6 +1052,14 @@ const server = http.createServer(async (req, res) => {
10521052 // so a write authors as THAT DID whoever is signed in — and sign-in admits any DID that can prove
10531053 // a delegation from its own #atproto key. Without this, any atproto account posts as wj62.
10541054 if (req.method === 'POST' && !OPEN_POST.has(path)) {
1055 // ⛔ SAY "READ-ONLY" BEFORE ASKING AN HPP THAT ISN'T THERE. A lens instance (stortron) has no
1056 // write transport, so every hpp call throws — including the device-revocation check. Fail-closed
1057 // then surfaced as "this device has been revoked" to a signed-in user whose device was perfectly
1058 // fine. Reported live on favela.7rnx.net. The refusal is right; the reason was a fabrication.
1059 if (!writeTransportConfigured(process.env)) {
1060 console.log(JSON.stringify({ msg: 'write refused', path, viewer: viewer ? viewer.did : null, status: 403, reason: 'read-only instance' }))
1061 return json(res, 403, JSON.stringify({ error: 'this instance is read-only — it serves feeds and cannot author records' }))
1062 }
10551063 const ctx = { identity: viewer }
10561064 const isDeviceRevoked = (k) => writer.isDeviceRevoked(k)
10571065 try {
lib/check.js +11 -5
……@@ -88,12 +88,18 @@ export function extractIdentity (req, sessions) {
8888 * right for a write: there, an unreachable registry must not wave through a device that may have been
8989 * revoked. Hence `failOpen`, defaulting to pgv3's behaviour and flipped by the write guards.
9090 */
91export const UNKNOWN = 'unknown'
92
9193export async function isRevoked ({ subKey, isDeviceRevoked, failOpen = true } = {}) {
9294 if (!subKey || typeof isDeviceRevoked !== 'function') return false
9395 try {
9496 return !!(await isDeviceRevoked(subKey))
9597 } catch {
96 return !failOpen // fail-open → "not revoked"; fail-closed → treat as revoked
98 // ⛔ A CHECK THAT COULD NOT RUN IS NOT A POSITIVE REVOCATION. This returned `true` on the write
99 // path, so a read-only instance — one with no hpp to ask — told a signed-in user "this device has
100 // been revoked". Their device was fine; nobody could check. Reported live on favela.7rnx.net.
101 // Fail-closed still DENIES; it must simply not assert something untrue about the user's device.
102 return failOpen ? false : UNKNOWN
97103 }
98104}
99105
……@@ -104,9 +110,9 @@ export async function isRevoked ({ subKey, isDeviceRevoked, failOpen = true } =
104110export async function requireIdentity (ctx, opts = {}) {
105111 const identity = ctx && ctx.identity
106112 if (!identity || !identity.did) throw new AuthError('authentication required')
107 if (await isRevoked({ subKey: identity.subKey, ...opts })) {
108 throw new AuthError('this device has been revoked', 403)
109 }
113 const verdict = await isRevoked({ subKey: identity.subKey, ...opts })
114 if (verdict === UNKNOWN) throw new AuthError('device status could not be verified', 403)
115 if (verdict) throw new AuthError('this device has been revoked', 403)
110116}
111117
112118/**
……@@ -174,4 +180,4 @@ export async function mayWrite (ctx, opts = {}) {
174180 }
175181}
176182
177export default { extractToken, extractIdentity, requireIdentity, requireMember, requireRepoOwner, mayWrite, isRevoked, AuthError, COOKIE_NAME }
183export default { UNKNOWN, extractToken, extractIdentity, requireIdentity, requireMember, requireRepoOwner, mayWrite, isRevoked, AuthError, COOKIE_NAME }
package.json +1 -1
……@@ -1,6 +1,6 @@
11{
22 "name": "favela-web",
3 "version": "0.3.56",
3 "version": "0.3.57",
44 "description": "favela-web \u2014 the web/SSR surface of Favela, the native P2P-local, community-owned social client + appview over the self-custody atproto stack (hyper-proto-peer). One of the Favela surfaces (alongside favela-cli and favela-native). bsky is an optional POSSE export, not the home. See DESIGN.md.",
55 "type": "module",
66 "main": "bin/favela-web.js",
test/check.test.mjs +14 -2
……@@ -9,7 +9,7 @@
99import { SessionStore } from '../lib/sessions.js'
1010import {
1111 AuthError, COOKIE_NAME, extractToken, extractIdentity,
12 isRevoked, requireIdentity, requireMember, requireRepoOwner, mayWrite
12 isRevoked, UNKNOWN, requireIdentity, requireMember, requireRepoOwner, mayWrite
1313} from '../lib/check.js'
1414
1515let pass = 0
……@@ -59,7 +59,11 @@ const ID = { did: DID, subKey: 'beef' }
5959 // reader out, but it must not wave a WRITE through either.
6060 const boom = async () => { throw new Error('registry unreachable') }
6161 eq(await isRevoked({ subKey: 'beef', isDeviceRevoked: boom }), false, 'read path FAILS OPEN on a registry error (pgv3 behaviour)')
62 eq(await isRevoked({ subKey: 'beef', isDeviceRevoked: boom, failOpen: false }), true, 'write path FAILS CLOSED on the same error')
62 // ⛔ A CHECK THAT COULD NOT RUN IS NOT A POSITIVE REVOCATION. Returning `true` here made the write
63 // path answer "this device has been revoked" to a user whose device was fine — the real cause was a
64 // read-only instance with no hpp to ask. It still DENIES; it must just say something true.
65 eq(await isRevoked({ subKey: 'beef', isDeviceRevoked: boom, failOpen: false }), UNKNOWN,
66 'write path reports UNKNOWN, not revoked, when the check itself fails')
6367}
6468
6569// ── requireIdentity ─────────────────────────────────────────────────
……@@ -70,6 +74,14 @@ const ID = { did: DID, subKey: 'beef' }
7074 await requireIdentity({ identity: ID })
7175 ok(true, 'a signed-in identity passes')
7276 await rejects(() => requireIdentity({ identity: ID }, { isDeviceRevoked: async () => true }), 403, 'a REVOKED device is refused even with a valid session')
77 // Both deny — but a user must never be told they were revoked when nobody could check.
78 try {
79 await requireIdentity({ identity: ID }, { isDeviceRevoked: async () => { throw new Error('no hpp') }, failOpen: false })
80 ok(false, 'an unverifiable device should be refused')
81 } catch (e) {
82 ok(/could not be verified/i.test(e.message) && !/revoked/i.test(e.message),
83 `an unverifiable device is refused WITHOUT claiming revocation (got: ${e.message})`)
84 }
7385}
7486
7587// ── requireMember — the seat for the invite gate ─────────────────────