Basekick-Labs / Basekick-Labs/arc

security(rbac): table-reference dedup key case-folds while the permission check does not, so one grant authorizes reads of two measurements

Open
#827 0 comments 0 reactions 0 assignees View on GitHub
bug security
Dominant language
Go
Stars
677
Forks
53
Avg merge
9h 14m
Merged PRs (30d)
164

Description

## Summary

The RBAC table-reference dedup key is lower-cased, but the permission check that consumes those references compares **case-sensitively**. A query naming the same measurement in two different cases is deduped to one reference, so one grant is checked while two measurements are read.

Split out of #750 (item 2) so it can be milestoned and closed on its own.

## The mismatch

The dedup key folds case — `internal/api/query.go:1225` and `:1274`:

```go
tableName := resolve(sql[matchIdx[2]:matchIdx[3]])
table := strings.ToLower(tableName)
```

The `TableReference` that reaches `CheckPermissionsBatch` keeps the **original** case, and `matchPattern` (`internal/auth/rbac_manager.go:1967`) compares case-sensitively:

```go
func matchPattern(pattern, value string) bool {
// Exact wildcard
if pattern == "*" {
return true
}
// Prefix wildcard (e.g., "prod_*" matches "prod_us", "prod_eu")
if strings.HasSuffix(pattern, "_*") {
...
```

## Repro shape

```sql
SELECT * FROM cpu WHERE x IN (SELECT y FROM CPU)
```

`cpu` and `CPU` fold to one dedup key, so the permission for one is checked, and both are read.

This matters because **object keys are case-sensitive on S3**: `cpu` and `CPU` are two distinct measurements with two separate grants, not two spellings of one. A principal granted `cpu` reads `CPU` without a grant for it.

## Why it is security-relevant

The authorization decision is made on a set that is smaller than the set actually read. Any measurement whose name differs only by case from one the principal *is* granted becomes readable.

Distinct from #740, which covers spoke identifiers rather than table references.

## Suggested fix

Make the two sides agree. Either:

1. **Preserve case in the dedup key** — dedup on the original `tableName` so distinct spellings stay distinct references and each is checked. Safest: it cannot under-check, only over-check.
2. Fold case in `matchPattern` too — but that changes grant semantics against case-sensitive object storage, so it is the wrong direction.

Option 1 is preferred: the check set should never be smaller than the read set.

## Test plan

- [ ] Test asserting `SELECT * FROM cpu WHERE x IN (SELECT y FROM CPU)` checks permissions for **both** `cpu` and `CPU`
- [ ] Test asserting a principal granted only `cpu` is denied that query
- [ ] Same for the JOIN path (`query.go:1274`)
- [ ] Regression test must FAIL pre-fix (revert-run-restore)

Split from #750.

Contributor guide

Open the contributing guide

Research direction

Start in internal/api/query.go at lines 1225 and 1274, then read TableReference handling through CheckPermissionsBatch and matchPattern in internal/auth/rbac_manager.go:1967. Add regression coverage for mixed-case subquery and JOIN references, including denial when only cpu is granted. Done means both cpu and CPU are checked independently and the pre-fix behavior fails the regression tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.