HarperFast / HarperFast/harper
Non-array permission.operations throws during user-cache load, breaking authentication instance-wide (reachable via v4→v5 upgrade)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
Found by @cb1kenobi reviewing the Studio roles UI (HarperFast/studio#1628), traced against v5.2.2 (`d46ea1f30`) and confirmed identical in v5.0.0 and v5.1.0.
## Summary
A role whose `permission.operations` is **not an array** makes the user-cache load throw, which fails authentication for **every user on the instance** — not just the holder of that role.
## The path
`security/user.ts`, in the loop that builds the user cache:
```ts
for (let user of users) {
user = _.cloneDeep(user);
user.role = roleMapObj[user.role];
appendSystemTablesToRole(user.role);
cacheExpandedOperationsPerms(user.role); // ← here
userMap.set(user.username, user);
}
```
```ts
function cacheExpandedOperationsPerms(userRole: UserRole) {
if (!userRole?.permission?.operations) return; // truthiness only
userRole.permission._expandedOperations = expandOperationsPerms(userRole.permission.operations);
}
```
`expandOperationsPerms` (`utility/operationPermissions.ts`) does `for (const item of operations)`, so any non-array truthy value throws `TypeError: operations is not iterable`. That rejects `listUsers()` → `setUsersWithRolesCache` → `findAndValidateUser`, i.e. auth for the whole instance.
Reproduced for `{tables:{…}}`, `{}`, and `true`. (A bare **string** doesn't throw — it iterates per character and silently expands to garbage operation names, which is its own small bug.)
## How a role gets into that state
`add_role`/`alter_role` reject it today (`OPERATIONS_MUST_BE_ARRAY`), so not through the API. The reachable path is a **v4 → v5 upgrade**: `operations` only became a reserved database name in #1016, so a v4 role granting a database named `operations` carries `permission.operations = { tables: … }` forward. Nothing rewrites it on upgrade, and the first cache load after a user holds that role takes auth down.
Only roles **assigned to a user** are expanded, so an orphaned role is inert until someone assigns it — which makes the failure look unrelated to the change that triggered it.
## Suggested fix
Make the guard shape-aware rather than truthiness-based, and fail soft for a value that can't be expanded:
```ts
if (!Array.isArray(userRole?.permission?.operations)) return;
```
That leaves the role unexpanded; `verifyPerms` then hits the same non-iterable value at request time, so it would want the same treatment (or a normalization at upgrade). Deciding what a malformed allowlist *means* is the real question — deny-all is the safe reading, but silently ignoring it would also be defensible given the value predates the feature. Either beats an instance-wide auth outage.
Worth an upgrade-time check too: scanning `hdb_role` for a non-array `operations` during the v4 → v5 migration would catch it before the instance restarts.
Studio (#1628) now warns on such a role and names assignment as the trigger, but it can only do that for someone who can still log in.
---
🤖 Filed by Claude on behalf of @dawsontoth
Contributor guide
Research direction
Start in security/user.ts at setUsersWithRolesCache and read utility/operationPermissions.ts, then trace verifyPerms and the v4→v5 upgrade path. Reproduce with a role whose permission.operations is an object or true and a user assigned to it. Done means malformed operations no longer cause instance-wide authentication failure and their request-time and upgrade behavior is explicitly handled.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- authentication, backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100