Repository navigation
storage(#7642): CI gate for storage-backend deps outside tinystoragedrivers - #7700
Conversation
Advance the vendored tinywallet submodule to a newer commit. Auto-committed-on: dragonfly
Update the vendored tinywallet submodule to the latest upstream commit. Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
The storage-owner check now runs as part of the rust:layout and storage:bypass scripts so ownership violations are caught in the existing CI paths. The script itself was reworked to deduplicate violations, print the report, and exit with a proper status code, and cargo metadata no longer passes --no-deps so dependency packages are included in the analysis. Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (3)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ebaa5d08f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "rust:module-boundaries": "node scripts/ci/check-module-boundaries.mjs", | ||
| "rust:module-boundaries:complete": "node scripts/ci/check-module-boundaries.mjs --require-complete", | ||
| "rust:layout": "node scripts/ci/check-openhuman-rust-layout.mjs && node scripts/ci/check-crate-chain.mjs && node scripts/ci/check-storage-bypass.mjs", | ||
| "rust:layout": "node scripts/ci/check-openhuman-rust-layout.mjs && node scripts/ci/check-crate-chain.mjs && node scripts/ci/check-storage-bypass.mjs && node scripts/ci/check-storage-owner.mjs", |
There was a problem hiding this comment.
Wire the owner check into the actual CI plan
Checked .github/workflows/ci-lanes.yml and scripts/ci/self-hosted/lanes-plan.mjs: PR CI executes check-openhuman-rust-layout.mjs directly at lines 189–192 and check-crate-chain.mjs at 231–233, and no workflow invokes pnpm rust:layout. Consequently manifest/dependency PRs never run this owner checker on repository metadata (the scripts lane only runs its fixture-based unit test), so the advertised gate cannot enforce the storage-driver ownership invariant; add it to the lane plan and arm it in .github/ci-paths-filter.yml.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| if (FORBIDDEN_CRATE_NAME.test(pkg.name)) { | ||
| out.push({ package: pkg.name, kind: 'crate-name', detail: `crate name matches ${FORBIDDEN_CRATE_NAME}` }); | ||
| } | ||
| if (Object.hasOwn(allowlist, pkg.name)) continue; |
There was a problem hiding this comment.
Scope reader exemptions to their justified dependency
If tinychannels, openhuman-app, or tinymemory-integrations later adds any other forbidden dependency (for example mongodb or object_store), this package-level continue skips the whole dependency loop, even though the recorded exception reasons only justify their read-only SQLite use. Such a real backend would pass the new gate; encode exemptions as package/dependency pairs and continue checking all other dependencies.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| "dep:audit": "bash scripts/dep-audit/run.sh", | ||
| "agent:runtime-boundary": "node scripts/ci/check-agent-runtime-boundary.mjs", | ||
| "storage:bypass": "node scripts/ci/check-storage-bypass.mjs", | ||
| "storage:owner": "node scripts/ci/check-storage-owner.mjs", |
There was a problem hiding this comment.
Update the command documentation with the owner gate
This adds a public storage:owner command and expands pnpm rust:layout, but scripts/ci/README.md has no entry for check-storage-owner.mjs and its local-running section still says only the layout and crate-chain checks run through that command; scripts/README.md likewise omits the new entry point. Update those command indexes with this contract change so contributors can discover and diagnose the new Cargo-metadata gate.
AGENTS.md reference: AGENTS.md:L773-L773
Useful? React with 👍 / 👎.
Tiny Sweeper reviewAdds a new CI gate that fails when a first-party package outside tinystoragedrivers* declares a forbidden storage-backend dependency or is named *-sqlite/*-mongodb/*-file. Across the critique, security, tests, commits, and description lanes no findings were reported, and the change is described as sound and safe to merge; note that the code index is behind this pull request (indexed at 7f4e63a), so retrieved context may be out of date, and one file (scripts/ci/check-storage-owner.mjs) could not be reviewed in the critique lane. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe pull request adds scripts/ci/check-storage-owner.mjs, a storage-owner CI gate that reads cargo metadata for the root workspace and crates/openhuman-app and fails when a first-party package outside tinystoragedrivers* declares a backend dependency or is named *-sqlite/*-mongodb/*-file. It wires the gate into the rust:layout script and adds a storage:owner script in package.json, and ships a colocated node:test self-test in scripts/__tests__/check-storage-owner.test.mjs that exercises the parsing, the allowlist, the report formatter and the CLI exit codes (0/1/2). No Rust changed. Features
Tests
FindingsNo active actionable findings. Could not review: scripts/ci/check-storage-owner.mjs Before merge
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: scripts/ci/check-storage-owner.mjs.
$0.0030 · 407,894 in / 37,068 out · 98,560 cached (24%) · ladder/vectors-oai3, deepseek-v4.1-flash · 324 embedded
critique: $0.0023 · 170,970 in / 32,847 out · 54,400 cached (32%) · deepseek-v4.1-flash
security: $0.0005 · 139,594 in / 1,728 out · 28,928 cached (21%) · deepseek-v4.1-flash
tests: $0.0001 · 59,849 in / 893 out · 7,808 cached (13%) · deepseek-v4.1-flash
description: $0.0001 · 32,374 in / 651 out · 7,424 cached (23%) · deepseek-v4.1-flash
Summary
scripts/ci/check-storage-owner.mjs, wired intopnpm rust:layout(andpnpm storage:owner).cargo metadatafor the root workspace andcrates/openhuman-app, and fails when a first-party crate (path or git source) outsidetinystoragedrivers*depends onrusqlite,libsqlite3-sys,mongodb,object_store,sqlx,redb,sledorrocksdb, or is named*-sqlite/*-mongodb/*-file. crates.io crates are ignored.tinychannels(iMessage chat.db reader),openhuman-app(imessage scanner),tinymemory-integrations(legacy-import).scripts/__tests__/check-storage-owner.test.mjs(8 tests, pass).Problem
Part of #7522: nothing stopped a storage backend dependency from creeping back in outside tinystoragedrivers.
Solution
Declared dependencies (any kind, optional included) of every first-party package in the resolved metadata are checked, so vendored crates are covered. No temporary allowlist entries were needed: the #7640 and #7641 offenders are already gone on upstream/main, so there is nothing to remove later.
Verification on this branch (
cargo tree -i, depth 1):node scripts/ci/check-storage-owner.mjsexits 0;check-openhuman-rust-layout.mjsandcheck-crate-chain.mjspass. No Rust changed, so no cargo builds were run.Submission Checklist
Impact
CI only.
crates/openhuman-app/Cargo.lockis left as upstream (cargo metadata locally wants a tinychannels-bus lock bump; unrelated).Related
Closes #7642
Summary by CodeRabbit