HarperFast / HarperFast/harper

export_local / export_to_s3 skip their requires_su gate when the nested search_operation is SQL

Open
#2,305 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 2h
Merged PRs (30d)
205

Description

## Mechanism

`chooseOperation` (`server/serverHelpers/serverUtilities.ts`) branches on whether the request carries SQL:

```js
if (json.operation === 'sql' || (json.search_operation && json.search_operation.operation === 'sql')) {
// operations-allowlist check + AST table/scope check
} else if (...) {
// verifyPerms
}
```

The two are **mutually exclusive**, and `requires_su` is enforced only inside `verifyPerms` (`utility/operation_authorization.ts:869`). `requiredPermissions` registers both export operations with `requiresSu = true` (`utility/operation_authorization.ts:416-422`), and `dataLayer/export.ts` performs no super_user check of its own.

So an ordinary authenticated role reaches a super_user-only operation by nesting a `sql` search_operation. The AST check still bounds *what* may be selected; it does not gate *whether the operation runs*.

`export_local` writes to a caller-supplied `path`, and `export_to_s3` uploads to caller-supplied bucket credentials.

## Status

Pre-existing; not introduced by #2202. Reported there by @cb1kenobi with runtime confirmation against that branch, and it is item 7 of that PR's description. #2217 approaches the same branch from the other side.

The exploit shape is already described publicly in https://github.com/HarperFast/harper/pull/2202 — filing this in the open follows that, rather than re-deciding disclosure unilaterally. **If this should instead be a private advisory, say so and I will move it; the affected shipped versions have not been established.**

## Fix direction

The branch needs the `requires_su` gate that `verifyPerms` provides — either by running both gates for job operations carrying nested SQL, or by hoisting the `requires_su` check ahead of the branch. Both change authorization outcomes for existing roles, so it wants a release note.

Related: #2215, #2216 (the two other deferred gaps in this same dispatch branch).

Contributor guide

Open the contributing guide

Research direction

Start by tracing chooseOperation in server/serverHelpers/serverUtilities.ts alongside verifyPerms and requiredPermissions in utility/operation_authorization.ts, then confirm how dataLayer/export.ts handles export_local and export_to_s3. Done means nested SQL requests cannot bypass the requires_su gate, with regression coverage for both operations and a release note for the authorization change.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
authorization, backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.