From bf7d716d1bf6a8093e5c66275156ccbc5ca4963b Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Sat, 2 May 2026 19:25:52 +0200 Subject: [PATCH] forbidden_apis: add `.restore(` and document why `.append(` / `.delete(` are excluded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged that the guard misses `ds.append() / ds.delete() / ds.restore()`. `.restore(` is added — Lance-specific (no false positives in the workspace). `.append(` and `.delete(` stay excluded with a documenting comment: * `.append(` over-matches `Vec::append`, `String::append`, every `arrow_array::xxxArrayBuilder::append` (30+ legit uses across `exec/mutation.rs`, `loader/jsonl.rs`, `exec/projection.rs`). * `.delete(` over-matches `ObjectStore::delete` (used in `storage.rs`, `db/schema_state.rs`, `db/omnigraph.rs:1277` for staging-file cleanup) and would require many `// forbidden-api-allow:` sentinels for legitimate uses. The remaining bypass route — engine code that imports `lance::Dataset` and calls `ds.append(reader, params)` — is bounded by: 1. The trait surface itself (sealed, only-callable-via-trait once Phase 1b call-site conversion completes). 2. The PR-review process catching new `lance::Dataset` imports in non-storage files. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/omnigraph/tests/forbidden_apis.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/omnigraph/tests/forbidden_apis.rs b/crates/omnigraph/tests/forbidden_apis.rs index 9cbe20b..055d96a 100644 --- a/crates/omnigraph/tests/forbidden_apis.rs +++ b/crates/omnigraph/tests/forbidden_apis.rs @@ -75,6 +75,19 @@ const FORBIDDEN_PATTERNS: &[&str] = &[ ".update_columns(", ".drop_columns(", ".truncate_table(", + // `.restore(` is Lance-specific (no other library in this workspace + // exposes a `.restore(` method); safe to ban without false-positive + // risk. Used to revert a Lance dataset to a prior version — never + // an operation engine code should perform directly. + ".restore(", + // NOT included: `.append(`, `.delete(`, `.write(`. Each over-matches + // legitimate non-Lance uses (`Vec::append`, `String::append`, arrow + // array `BuilderArray::append`, `ObjectStore::delete`, etc.). + // Engine code calling `ds.append(reader, params)` against an + // imported `lance::Dataset` is the residual bypass route this guard + // does NOT catch — but the trait surface itself is the primary + // enforcement (sealed + only-callable-via-trait once Phase 1b + // call-site conversion completes), so this gap is bounded. ]; /// Files exempt from the guard. These are the legitimate storage-layer