HarperFast / HarperFast/harper

Non-array permission.operations throws during user-cache load, breaking authentication instance-wide (reachable via v4→v5 upgrade)

Open
#2,194 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.