Basekick-Labs / Basekick-Labs/arc
More lossy encodings that become paths, keys or authorization decisions
- 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
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