feat(security): provenance-keyed sandbox tiers for the app capability model#1574
Conversation
Classify every userspace app into one of four provenance tiers (first-party, ai-generated, user-uploaded, unknown) and add a single source of truth mapping each tier to its default capability ceiling in tinyagentos/userspace/capabilities.py. The ceiling is what an app holds without user consent; anything beyond it still goes through the existing app-permission grant flow. Add the provenance column to the userspace app store with a migration that backfills legacy rows from their existing trust value (first-party stays first-party, everything else becomes user-uploaded), so existing installs classify sensibly with no manual step. Boot-seeded bundled apps now record provenance="first-party" explicitly.
…on API
Wire the provenance ceiling into the places that actually decide what an
app can do:
- routes/userspace_apps.py: the broker's granted-capability set now
starts from the app's provenance ceiling instead of a hard-coded
trust=="first-party" check, generalising the existing bypass to all
four tiers with no behaviour change for first-party or the gated
network/agent/llm/memory capabilities. The public install endpoint
accepts an optional provenance ("ai-generated" or "user-uploaded"
only -- never first-party) and defaults to "user-uploaded" for a
bare side-load. The bundle CSP also tightens img-src further for the
unknown tier.
- routes/app_permissions.py: GET .../permissions now reports an app's
provenance and ceiling, and request-consent flags a capability outside
the ceiling as needing consent for any classified userspace app (a
first-party app skips consent entirely, matching its full ceiling; an
ai-generated or user-uploaded app now correctly needs consent even for
storage capabilities that were previously free for everyone). Callers
that aren't a classified userspace app (native features using this
same grant ledger) keep their exact previous behaviour.
- routes/apps.py: the optional first-party frontend apps now also carry
a provenance in their installed_apps metadata and the catalog response.
The raw broker dispatch (handle_capability) still treats storage
capabilities as always-on regardless of tier, unchanged from before --
several existing tests exercise free storage access on a community-trust
app with no grant, and tightening that shared path would break real,
already-working installs. That gap is closed at the consent-request
layer instead; fully closing it at the broker dispatch layer is a
follow-up.
Docs-Reviewed: no desktop app was added or removed by this change (only
a co-located test file under desktop/src/apps/appstudio/__tests__/); the
provenance model is new and undocumented elsewhere in the repo, so there
is no existing doc to update.
Pass an app's provenance tier and granted capabilities down from fetchUserspaceApps/toAppManifest into SandboxedAppWindow. The iframe sandbox attribute stays "allow-scripts" for every tier -- it is already the tightest value that still lets the app run, and adding tokens like allow-forms/allow-popups/allow-same-origin for any tier would loosen an existing restriction, which this change must not do -- but the derivation is now a named, provenance-aware, testable function instead of a bare constant, and the iframe carries a data-provenance attribute. Non-first-party apps get a small corner badge naming their provenance and whether the network capability is currently granted, so a user looking at an AI-generated or user-uploaded app's window can see e.g. "AI-generated - Network blocked" at a glance.
App Studio publishes are, by definition, ai-generated -- give the Publish view's app identity header a small badge saying so, next to where its requested capabilities are already listed. This is the other surface the provenance model calls for besides the app window itself: the point where a user reviews an app's permissions before it goes live.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThis PR adds a provenance classification system (first-party, ai-generated, user-uploaded, unknown) for userspace apps, computing capability ceilings per tier. Provenance is persisted in storage, enforced in permission/broker/CSP routes, and surfaced via badges and sandbox attributes in the desktop UI. ChangesProvenance capability system
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AppPermissionsRoute
participant UserspaceAppsRoute
participant CapabilitiesModule
Client->>AppPermissionsRoute: POST /api/apps/{app_id}/request-consent
AppPermissionsRoute->>AppPermissionsRoute: _resolve_provenance(app_id)
AppPermissionsRoute->>CapabilitiesModule: capability_allowed(provenance, cap, granted)
CapabilitiesModule-->>AppPermissionsRoute: allowed / pending
Client->>UserspaceAppsRoute: GET /api/userspace-apps/{app_id}/broker
UserspaceAppsRoute->>CapabilitiesModule: capability_ceiling(provenance)
CapabilitiesModule-->>UserspaceAppsRoute: ceiling capabilities
UserspaceAppsRoute-->>Client: granted capabilities response
Possibly related PRs
Poem A rabbit sniffs each app anew, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| capability: string, | ||
| granted: readonly string[], | ||
| ): boolean { | ||
| return PROVENANCE_CEILINGS[provenance].includes(capability) || granted.includes(capability); |
There was a problem hiding this comment.
WARNING: hasCapability uses an exact-string .includes against PROVENANCE_CEILINGS, but the backend's capability_allowed (tinyagentos/userspace/capabilities.py:176) computes _namespace(capability) (the a.b prefix) and checks that against the ceiling. For a sub-capability like app.kv.get the backend treats it as app.kv and allows it for first-party apps; this frontend check would say false and the badge would report the app as not having the capability it actually has. Either match the namespace semantics (capability.split('.')[0] + '.' + capability.split('.')[1]) or narrow the docstring to "exact match only -- sub-capabilities not shown".
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ns = _namespace(capability) | ||
| if ns in capability_ceiling(provenance): | ||
| return True | ||
| granted_set = set(granted) if not isinstance(granted, (set, frozenset)) else granted |
There was a problem hiding this comment.
SUGGESTION: granted_set = set(granted) if not isinstance(granted, (set, frozenset)) else granted returns the caller's set/frozenset reference directly without copying. Read-only callers here are fine, but the asymmetry between the two branches (one returns a fresh set, one returns the caller's) invites future mutation bugs. Normalise via set(granted) if not isinstance(granted, (set, frozenset)) else set(granted) and drop the caller reference.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # tell "never classified" apart from an explicitly-set tier, then | ||
| # derive a sensible value per row from its existing trust column. | ||
| await self._db.execute( | ||
| "ALTER TABLE userspace_apps ADD COLUMN provenance TEXT NOT NULL DEFAULT ''" |
There was a problem hiding this comment.
SUGGESTION: The new provenance migration runs ALTER TABLE ... ADD COLUMN followed by an UPDATE backfill, but a commit only happens after the UPDATE. SQLite DDL auto-commits, but aiosqlite runs inside an explicit transaction started by BaseStore.init() -- if init later wraps the migration in a transaction that fails, the ALTER will already be committed while the UPDATE may or may not be. Add an explicit await self._db.commit() between the ALTER and the UPDATE so each statement is its own atomic step, matching the existing trust-column pattern a few lines above.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge All three findings from the previous review have been resolved in the new commits:
No new issues were introduced by the incremental changes ( Files Reviewed (4 changed files in this incremental diff)
Previous Review Summary (commit 07de06b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 07de06b)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (13 files)
Reviewed by minimax-m3 · Input: 54.8K · Output: 7.4K · Cached: 1.3M |
… commit) - SandboxedAppWindow.hasCapability now matches on the capability namespace (app.kv.get -> app.kv) like the backend's capability_allowed, instead of an exact-string includes, so a sub-capability is recognised against its ceiling entry. - capabilities.capability_allowed always builds a fresh set from granted rather than aliasing the caller's set in one branch. - store.py commits the provenance ALTER TABLE before the backfill UPDATE, keeping DDL and DML in separate transactions to match the trust column's established migration pattern.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
desktop/src/lib/userspace-apps.ts (1)
9-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
AppProvenanceanddefaultProvenanceForTrustare duplicated verbatim inSandboxedAppWindow.tsx.Both the type union and the helper function are independently redefined identically in
desktop/src/apps/SandboxedAppWindow.tsx(no import between the two files). This file's own comment says it "mirrors" the backend'sPROVENANCE_TIERS, but doesn't mention it's also duplicating the sibling frontend file — a future tier addition risks updating only one copy and causing type drift between the two.Consider extracting both into a small shared module (e.g.
desktop/src/lib/app-provenance.ts) that bothuserspace-apps.tsandSandboxedAppWindow.tsximport — cheap enough to import eagerly without defeatingSandboxedAppWindow's lazy-loading.♻️ Suggested consolidation sketch
// desktop/src/lib/app-provenance.ts export type AppProvenance = "first-party" | "ai-generated" | "user-uploaded" | "unknown"; export function defaultProvenanceForTrust(trust?: string): AppProvenance { return trust === "first-party" ? "first-party" : "user-uploaded"; }-/** Where an app's code came from. See tinyagentos/userspace/capabilities.py - * PROVENANCE_TIERS for the single source of truth this mirrors. */ -export type AppProvenance = "first-party" | "ai-generated" | "user-uploaded" | "unknown"; +import type { AppProvenance } from "./app-provenance"; +export { defaultProvenanceForTrust } from "./app-provenance"; +export type { AppProvenance };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/lib/userspace-apps.ts` around lines 9 - 31, The `AppProvenance` union and `defaultProvenanceForTrust` helper are duplicated in `userspace-apps.ts` and `SandboxedAppWindow.tsx`, which can drift over time. Extract both into a shared module such as `app-provenance.ts` and update both `UserspaceAppRow` consumers and `SandboxedAppWindow` to import from that single source so the provenance tiers and default mapping stay consistent.tinyagentos/userspace/capabilities.py (1)
167-181: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHarden
grantedtyping against accidental string input.
granted: objectaccepts anything; a caller accidentally passing a bare string (itself technically iterable) would silently iterate per-character viaset(granted)instead of raising, causing every membership check to fail silently (fail-closed, but hard to debug). All current call sites pass lists/sets, so this isn't actively triggered, but nothing guards against future misuse.♻️ Suggested guard
def capability_allowed(provenance: str, capability: str, granted: object) -> bool: ns = _namespace(capability) if ns in capability_ceiling(provenance): return True - granted_set = set(granted) + granted_set = {granted} if isinstance(granted, str) else set(granted) return ns in granted_set or capability in granted_set🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/userspace/capabilities.py` around lines 167 - 181, The capability_allowed function currently accepts granted as object and blindly converts it with set(granted), which can mis-handle accidental bare string inputs by iterating characters. Tighten the input handling in capability_allowed by adding an explicit guard against string-like granted values and only normalizing true collection inputs before the ns/capability membership checks. Keep the fix localized to capability_allowed and its granted_set construction so existing list/set callers continue to work unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@desktop/src/lib/userspace-apps.ts`:
- Around line 9-31: The `AppProvenance` union and `defaultProvenanceForTrust`
helper are duplicated in `userspace-apps.ts` and `SandboxedAppWindow.tsx`, which
can drift over time. Extract both into a shared module such as
`app-provenance.ts` and update both `UserspaceAppRow` consumers and
`SandboxedAppWindow` to import from that single source so the provenance tiers
and default mapping stay consistent.
In `@tinyagentos/userspace/capabilities.py`:
- Around line 167-181: The capability_allowed function currently accepts granted
as object and blindly converts it with set(granted), which can mis-handle
accidental bare string inputs by iterating characters. Tighten the input
handling in capability_allowed by adding an explicit guard against string-like
granted values and only normalizing true collection inputs before the
ns/capability membership checks. Keep the fix localized to capability_allowed
and its granted_set construction so existing list/set callers continue to work
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8653794f-b157-4e8f-9f1e-19cc2adbc013
📒 Files selected for processing (13)
desktop/src/apps/SandboxedAppWindow.tsxdesktop/src/apps/__tests__/SandboxedAppWindow.test.tsxdesktop/src/apps/appstudio/PublishView.tsxdesktop/src/apps/appstudio/__tests__/PublishView.test.tsxdesktop/src/lib/userspace-apps.tstests/test_app_provenance.pytests/test_provenance_enforcement.pytinyagentos/routes/app_permissions.pytinyagentos/routes/apps.pytinyagentos/routes/userspace_apps.pytinyagentos/userspace/capabilities.pytinyagentos/userspace/seed.pytinyagentos/userspace/store.py
1 participant
Summary
Classifies every app into one of four provenance tiers --
first-party,ai-generated,user-uploaded,unknown-- and maps each tier to a default capability ceiling, so an app's sandbox is shaped by where its code came from, not just a binary trusted/community flag.tinyagentos/userspace/capabilities.py: single source of truth.PROVENANCE_TIERS,PROVENANCE_CEILINGS(the default, no-consent capability set per tier),capability_ceiling(),capability_allowed(), anddefault_provenance_for_trust()for back-compat.tinyagentos/userspace/store.py: newprovenancecolumn onuserspace_apps, migrated in from the existingtrustcolumn for pre-existing rows (first-party stays first-party, everything else becomes user-uploaded).tinyagentos/routes/userspace_apps.py: the broker's granted-capability set is now built from the app's provenance ceiling instead of a hard-codedtrust == "first-party"check (same effective behaviour for first-party and for the gated network/agent/llm/memory capabilities, now expressed as one of four tiers instead of two). The public install endpoint accepts an optionalprovenance(ai-generatedoruser-uploadedonly) and defaults touser-uploaded. The bundle CSP tightensimg-srcfurther for theunknowntier.tinyagentos/routes/app_permissions.py:GET /api/apps/{id}/permissionsnow reportsprovenanceandceiling;request-consentnow correctly flags storage capabilities as needing consent for a classified ai-generated/user-uploaded app (previously free for everyone). Untracked app ids (native features using the same ledger) keep their exact previous behaviour.tinyagentos/routes/apps.py: the optional first-party frontend apps also carryprovenancein theirinstalled_appsmetadata and catalog response.desktop/src/apps/SandboxedAppWindow.tsx: acceptsprovenance+grantedCapabilities, renders a small corner badge for non-first-party apps ("AI-generated - Network blocked" etc). The iframesandboxattribute staysallow-scriptsfor every tier (already the tightest value; widening it for any tier would loosen an existing restriction).desktop/src/apps/appstudio/PublishView.tsx: an AI-generated badge next to the app identity, where its requested capabilities are already listed.Known scope boundary (disclosed, not a gap I missed)
The raw broker dispatch (
handle_capability) still treatsapp.kv/app.table/app.filesas always-on for every tier, unchanged from before. Several pre-existing tests (tests/userspace/test_e2e.py,tests/userspace/test_routes.py::test_broker_enforces_granted) exercise free storage access with no grant on a community-trust app installed through the public endpoint -- tightening that shared dispatch path would break those already-working installs. The ceiling IS enforced there for storage at the consent-request layer (request-consentnow flags it) and at the broker layer for the gated network/agent/llm/memory capabilities. Fully closing the storage gap at the broker dispatch layer is a natural follow-up once App Studio's publish flow is wired to actually useprovenance=ai-generated.Test plan
pytest tests/test_app_provenance.py tests/test_provenance_enforcement.py-- new tests for the ceiling table and route-level enforcementpytest tests/userspace tests/test_app_capabilities.py tests/test_routes_app_permissions.py tests/test_routes_userspace_apps.py tests/test_userspace_seed.py tests/test_userspace_store.py tests/test_routes_apps.py-- zero regressions across every affected existing suitecd desktop && npx vitest run-- full suite (278 files / 2322 tests) passescd desktop && npm run build-- succeedspython scripts/check_doc_gate.py diff-gate --base origin/dev-- clean (Docs-Reviewed trailer on the commit that adds a co-located test under desktop/src/apps/appstudio/**; no app was actually added/removed)Summary by CodeRabbit
New Features
Bug Fixes