Basekick-Labs / Basekick-Labs/arc

More lossy encodings that become paths, keys or authorization decisions

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

Description

Found while auditing #744. Filed together because they share one shape: a lossy transform whose output is then used as an identity. Ordered by severity.

## 1. Daily compaction flattens source keys into one temp directory

`internal/compaction/job.go:596`: `localPath := filepath.Join(tempDir, filepath.Base(fileKey))`, then `os.Create`, which truncates with no collision check.

The daily tier's candidate spans all 24 hour subdirectories (`daily.go:150-218`), and file basenames are `{measurement}_{YYYYMMDD}_{HHMMSS}_{nanos}.parquet` derived from wall-clock only (`ingest/arrow_writer.go:3668`, no node id, PID or random component). Two source files from different hours can share a basename. The result: one file's rows are duplicated into the output, the other's are dropped, and **both originals are marked compacted and deleted**.

Same shape as the already-closed #357.

## 2. RBAC dedup key case-folds while the permission check does not

`internal/api/query.go:1222` and `:1271` build the dedup key with `table := strings.ToLower(tableName)`, but the `TableReference` that reaches `CheckPermissionsBatch` keeps the original case, and `matchPattern` (`internal/auth/rbac_manager.go:1968`) compares case-sensitively.

So `SELECT * FROM cpu WHERE x IN (SELECT y FROM CPU)` checks the permission for `cpu` only, and reads both. Object keys are case-sensitive on S3, so those are two measurements with two separate grants.

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

## 3. `sanitizeDBForName`'s doc comment asserts an invariant that is false

`internal/compaction/manager.go:394-396` claims "no legal Arc database name may contain `.`". Its inputs are pseudo-databases from `expandNamespaces` (`manager.go:378`), and `validateSpokeID` (`internal/edgesync/receive.go:620-665`) rejects only a leading dot and `..`. So spoke `rocket.01` + database `telemetry` and spoke `rocket` + database `01.telemetry` both sanitize to `rocket.01.telemetry`.

Masked today because `jobID` also embeds the folded partition path, which differs. The comment should not be trusted as written.

## 4. `NewJob`'s fallback jobID uses the raw database name

`internal/compaction/job.go:229-232` builds a fallback jobID from `cfg.Database` without `sanitizeDBForName`, so a pseudo-database yields a jobID containing `/`, which `validateJobID` (`completion.go:266`) rejects and which `filepath.Join(TempDirectory, JobID)` turns into a nested directory. Latent only because `manager.go` always supplies the jobID, and it is the exact failure `manager.go:387-393` exists to prevent.

## 5. Arc performs the Azure separator fold it rejects keys for

`internal/api/query.go:3203`: `path = strings.ReplaceAll(path, "\\", "/")` in `extractDBMeasurementFromPath`. #743 rejects backslash in keys precisely because Azure treats it as a separator. The consumer here is tiering glob selection (`query.go:3057`), so the effect is a misrouted query rather than an authorization failure.

## 6. Smaller ones

- `internal/iceberg/exporter.go:820-826`: leading zeros folded by `Atoi` into `v{n}.metadata.json`.
- `internal/tiering/metadata.go:111,550` and `internal/ingest/arrow_writer.go:1704,1875`: `db + "/" + measurement` cache keys, safe only while a measurement cannot contain `/`, which edge-sync pseudo-databases already violate one level up.
- `internal/ingest/arrow_writer.go:474`: `%v` on a `[]string` in the schema cache key, so `["a b"]` and `["a","b"]` collide. Column names are never charset-validated.
- `internal/compaction/manifest.go:346-373`: the `GetFilesInManifests` warm path returns inputs only, dropping the `OutputPath` the cold path adds at `:396`, so for up to 30s after a rebuild `filterCandidateFiles` stops shielding in-flight compaction outputs.
- `internal/compaction/manifest.go:28-31`: `ManifestMaxAge` claims stale manifests are deleted; `:238-247` computes `isStale` and uses it for a log line only. `RELEASE_NOTES_2026.02.1.md:470` repeats the claim.
- `internal/storage/local.go:404` and `:414`: `AppendReader` has both a deferred `file.Close()` and an explicit one, so the file is closed twice.

Contributor guide

Open the contributing guide

Research direction

Start with the cited entry points in internal/compaction/job.go, internal/api/query.go, internal/auth/rbac_manager.go, and internal/compaction/manager.go, then trace the related edge-sync, tiering, manifest, exporter, ingest, and storage paths. Reproduce the listed collisions and run the focused tests for each subsystem; done means every lossy identity or path transformation has distinct handling and the documented invariants match validation behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, azure, go
Domain
backend, cloud, databases, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.