feat(userspace): trust-aware runtime for first-party packages (#89 P3b)#972
Conversation
…or first-party packages (#89 P3b) - store.py: add trust column (TEXT DEFAULT 'community') with _post_init migration (PRAGMA table_info + ALTER TABLE -- same pattern as knowledge_store and agent_registry_store); install() accepts trust kwarg - routes/userspace_apps.py: public install endpoint always writes trust='community' (first-party comes only from internal seed/P2 sig path); serve_bundle picks _BUNDLE_CSP_FIRST_PARTY vs _BUNDLE_CSP by app trust; broker route passes full GATED_CAPS set as granted for first-party apps, per-grant set for community apps - SandboxedAppWindow.tsx: accepts trust prop; posts taosTheme tokens into the iframe on load and on scheme changes for first-party apps only; community apps receive no theme injection - userspace-apps.ts: UserspaceAppRow gains optional trust field; toAppManifest threads it into the component closure for SandboxedAppWindow - taos-app-sdk.js: adds taos.theme.get() / taos.theme.subscribe(cb) backed by the taosTheme postMessage; community apps never receive messages so the API simply returns an empty object for them - tests: 10 new backend tests (store migration, public endpoint enforces community, CSP by trust, broker grants for first-party vs community, GATED_CAPS unit); 8 new frontend tests (theme injection on/off by trust, SDK theme message handling, trust field in toAppManifest)
…oncile with set_permissions security fix)
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? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a ChangesTrust-aware Userspace App Enforcement
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over Client,UserspaceAppStore: Install flow (trust always pinned to community)
Client->>InstallEndpoint: POST manifest zip
InstallEndpoint->>UserspaceAppStore: install(trust="community")
UserspaceAppStore-->>InstallEndpoint: stored
end
rect rgba(144, 238, 144, 0.5)
Note over Client,BundleEndpoint: Bundle serving (CSP selected by trust)
Client->>BundleEndpoint: GET /bundle/index.html
BundleEndpoint->>UserspaceAppStore: lookup app.trust
alt first-party
BundleEndpoint-->>Client: _BUNDLE_CSP_FIRST_PARTY header
else community
BundleEndpoint-->>Client: _BUNDLE_CSP_COMMUNITY header
end
end
rect rgba(255, 218, 185, 0.5)
Note over SandboxedAppWindow,SDK: Theme token push (first-party iframes only)
useThemeStore-->>SandboxedAppWindow: scheme change
SandboxedAppWindow->>SandboxedAppWindow: readThemeTokens() from :root
SandboxedAppWindow->>Iframe: postMessage taosTheme tokens
Iframe->>SDK: message event (taosTheme)
SDK->>SDK: store tokens, notify subscribers
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| await self._db.execute( | ||
| """INSERT INTO userspace_apps | ||
| (app_id, name, version, app_type, entry, icon, | ||
| permissions_requested, permissions_granted, enabled, installed_at) | ||
| VALUES (?,?,?,?,?,?,?,'[]',1,?) | ||
| permissions_requested, permissions_granted, enabled, installed_at, trust) | ||
| VALUES (?,?,?,?,?,?,?,'[]',1,?,?) | ||
| ON CONFLICT(app_id) DO UPDATE SET | ||
| name=excluded.name, version=excluded.version, | ||
| app_type=excluded.app_type, entry=excluded.entry, | ||
| icon=excluded.icon, | ||
| permissions_requested=excluded.permissions_requested""", | ||
| (app_id, name, version, app_type, entry, icon, | ||
| json.dumps(permissions_requested), int(time.time())), | ||
| json.dumps(permissions_requested), int(time.time()), trust), | ||
| ) | ||
| await self._db.commit() |
There was a problem hiding this comment.
⚠️ Security: Public install can overwrite first-party bundle, keeping elevated trust
The install() UPSERT now inserts trust in the VALUES list, but the ON CONFLICT(app_id) DO UPDATE SET ... clause does NOT update trust (store.py:60-65). Meanwhile extract_package writes the package files into apps_root/<manifest.id> with mkdir(exist_ok=True) and no existence/trust check, so it overwrites the files of any existing app with the same id (package.py:82-96).
Consequence: once P4/P2 seed a first-party app (e.g. id studio), an attacker who can reach the public POST /api/userspace-apps/install endpoint can upload a package whose manifest id collides with that first-party app. extract_package replaces the on-disk bundle with attacker code, and because the conflict path leaves trust untouched, the row stays trust='first-party'. The attacker's code is then served with the relaxed _BUNDLE_CSP_FIRST_PARTY and the broker grants it ALL GATED_CAPS (userspace_apps.py:191, 235-238). This directly defeats the PR's stated invariant that 'Trust is never self-declared' — trust is effectively inherited by id collision.
This is latent in P3b (no first-party apps exist yet) but the vulnerable pattern is introduced by this diff and must be closed before first-party apps are seeded.
Fix: make the conflict path reset trust to the supplied value (the public endpoint always passes 'community', so a public reinstall safely downgrades; the internal trusted path passes 'first-party' explicitly). Alternatively, reject a public reinstall when the existing row's trust is 'first-party'.
Update trust on conflict so a public reinstall (always trust='community') downgrades a colliding id, preventing inheritance of first-party trust.:
ON CONFLICT(app_id) DO UPDATE SET
name=excluded.name, version=excluded.version,
app_type=excluded.app_type, entry=excluded.entry,
icon=excluded.icon,
permissions_requested=excluded.permissions_requested,
trust=excluded.trust""",
Was this helpful? React with 👍 / 👎
|
Note Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually. Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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.
Inline comments:
In `@desktop/src/apps/__tests__/SandboxedAppWindow.test.tsx`:
- Around line 126-172: The test cases in the "SDK theme API (message handler)"
describe block are using inline mock handler implementations instead of testing
the actual SDK. Remove the inline handler logic and subscribers array from both
test cases. Instead, import and load the real SDK implementation (from
tinyagentos/userspace/sdk/taos-app-sdk.js), then dispatch the theme messages and
directly test the real window.taos.theme.get() and window.taos.theme.subscribe()
APIs. This ensures tests verify the actual SDK behavior rather than just mock
logic.
In `@desktop/src/lib/__tests__/userspace-apps.test.ts`:
- Around line 13-34: The test for trust propagation in toAppManifest is
incomplete. Currently it only verifies that mCommunity and mFirstParty produce
different component closures and have the correct category, but it doesn't
actually verify that the trust value is forwarded to SandboxedAppWindow. Add
test logic that calls the component factory functions (mCommunity.component and
mFirstParty.component) and inspects the props that would be passed to
SandboxedAppWindow to confirm that each component factory correctly captures and
passes through its respective trust value ("community" for mCommunity and
"first-party" for mFirstParty) to the SandboxedAppWindow component.
In `@tests/userspace/test_trust.py`:
- Around line 118-130: Add a new test function to validate that reinstalling an
existing first-party app via the public install endpoint downgrades its trust to
community. The test should first seed the database with an app having trust set
to "first-party" (using the same app_id "studio"), then call the public POST
/api/userspace-apps/install endpoint with the same package, and finally verify
that the stored trust value is downgraded to "community" rather than remaining
first-party. This complements the existing
test_public_install_endpoint_always_community function which only covers the
fresh-install scenario.
In `@tinyagentos/userspace/sdk/taos-app-sdk.js`:
- Around line 21-25: The type check on line 21 for `m.taosTheme` using `typeof
m.taosTheme === "object"` is too permissive because arrays also pass this check
in JavaScript, breaking the expectation that `_themeTokens` should be a plain
object token map. Modify the condition in the if statement to additionally
verify that `m.taosTheme` is not an array before storing it and notifying
subscribers through the callback loop. This ensures only plain objects are
accepted as valid theme payloads.
In `@tinyagentos/userspace/store.py`:
- Around line 61-67: The SQL upsert statement in the install_app method does not
update the trust column when a conflict occurs on app_id, which allows stale
first-party trust values to persist after reinstallation through the public
endpoint. Add trust=excluded.trust to the ON CONFLICT DO UPDATE SET clause
alongside the other excluded fields (name, version, app_type, entry, icon,
permissions_requested) to ensure the trust value is properly updated on each
installation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bea23f21-09e7-48e8-8384-9feaa3c00637
📒 Files selected for processing (8)
desktop/src/apps/SandboxedAppWindow.tsxdesktop/src/apps/__tests__/SandboxedAppWindow.test.tsxdesktop/src/lib/__tests__/userspace-apps.test.tsdesktop/src/lib/userspace-apps.tstests/userspace/test_trust.pytinyagentos/routes/userspace_apps.pytinyagentos/userspace/sdk/taos-app-sdk.jstinyagentos/userspace/store.py
| describe("SDK theme API (message handler)", () => { | ||
| it("taosTheme message is stored and accessible via taos.theme.get()", () => { | ||
| // Simulate the SDK's message handler inline (no iframe needed -- pure unit test). | ||
| let themeTokens: Record<string, string> = {}; | ||
| const subscribers: Array<(t: Record<string, string>) => void> = []; | ||
|
|
||
| const handler = (e: MessageEvent) => { | ||
| const m = e.data; | ||
| if (m && m.taosTheme && typeof m.taosTheme === "object") { | ||
| themeTokens = m.taosTheme; | ||
| for (const cb of subscribers) cb(themeTokens); | ||
| } | ||
| }; | ||
| window.addEventListener("message", handler); | ||
|
|
||
| const tokens = { "--color-accent": "#7c3aed", "--color-shell-bg": "#1a1b2e" }; | ||
| window.dispatchEvent(new MessageEvent("message", { data: { taosTheme: tokens } })); | ||
|
|
||
| expect(themeTokens).toEqual(tokens); | ||
|
|
||
| window.removeEventListener("message", handler); | ||
| }); | ||
|
|
||
| it("taosTheme subscribers are called on theme push", () => { | ||
| const received: Record<string, string>[] = []; | ||
| const subscribers: Array<(t: Record<string, string>) => void> = [ | ||
| (t) => received.push(t), | ||
| ]; | ||
|
|
||
| const handler = (e: MessageEvent) => { | ||
| const m = e.data; | ||
| if (m && m.taosTheme && typeof m.taosTheme === "object") { | ||
| for (const cb of subscribers) cb(m.taosTheme); | ||
| } | ||
| }; | ||
| window.addEventListener("message", handler); | ||
|
|
||
| window.dispatchEvent(new MessageEvent("message", { | ||
| data: { taosTheme: { "--color-accent": "#ff0000" } }, | ||
| })); | ||
|
|
||
| expect(received).toHaveLength(1); | ||
| expect(received[0]!["--color-accent"]).toBe("#ff0000"); | ||
|
|
||
| window.removeEventListener("message", handler); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
“SDK theme API” tests don’t execute the SDK implementation.
These tests re-implement handler logic inline, so they’ll still pass if tinyagentos/userspace/sdk/taos-app-sdk.js breaks. Please move/replace them with tests that load the real SDK and assert window.taos.theme.get() + subscribe() behavior directly.
🤖 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/apps/__tests__/SandboxedAppWindow.test.tsx` around lines 126 -
172, The test cases in the "SDK theme API (message handler)" describe block are
using inline mock handler implementations instead of testing the actual SDK.
Remove the inline handler logic and subscribers array from both test cases.
Instead, import and load the real SDK implementation (from
tinyagentos/userspace/sdk/taos-app-sdk.js), then dispatch the theme messages and
directly test the real window.taos.theme.get() and window.taos.theme.subscribe()
APIs. This ensures tests verify the actual SDK behavior rather than just mock
logic.
| it("passes trust='first-party' through to the component factory closure", async () => { | ||
| // The component factory must close over the trust value from the row so | ||
| // SandboxedAppWindow receives it. We can't render the component here (no | ||
| // DOM), but we can verify the factory captures the right trust by calling | ||
| // it and inspecting the SandboxedAppWindow props it would produce via the | ||
| // dynamic import. Instead, test the simpler invariant: community default. | ||
| const mCommunity = toAppManifest({ | ||
| app_id: "c", name: "C", icon: "", app_type: "web", version: "1", | ||
| enabled: 1, permissions_requested: [], permissions_granted: [], | ||
| trust: "community", | ||
| }); | ||
| const mFirstParty = toAppManifest({ | ||
| app_id: "fp", name: "FP", icon: "", app_type: "web", version: "1", | ||
| enabled: 1, permissions_requested: [], permissions_granted: [], | ||
| trust: "first-party", | ||
| }); | ||
| // Both should produce valid manifests in the userspace category. | ||
| expect(mCommunity.category).toBe("userspace"); | ||
| expect(mFirstParty.category).toBe("userspace"); | ||
| // The component functions are distinct closures (one per row). | ||
| expect(mCommunity.component).not.toBe(mFirstParty.component); | ||
| }); |
There was a problem hiding this comment.
Test doesn’t actually verify trust propagation.
Line 33 only proves two closures are different; it doesn’t prove trust is forwarded to SandboxedAppWindow. This can pass even if trust wiring regresses.
Suggested test tightening
- // The component functions are distinct closures (one per row).
- expect(mCommunity.component).not.toBe(mFirstParty.component);
+ const c = await mCommunity.component();
+ const fp = await mFirstParty.component();
+ const renderedC = c.default({ windowId: "w1" });
+ const renderedFP = fp.default({ windowId: "w2" });
+ expect(renderedC.props.trust).toBe("community");
+ expect(renderedFP.props.trust).toBe("first-party");🤖 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/__tests__/userspace-apps.test.ts` around lines 13 - 34, The
test for trust propagation in toAppManifest is incomplete. Currently it only
verifies that mCommunity and mFirstParty produce different component closures
and have the correct category, but it doesn't actually verify that the trust
value is forwarded to SandboxedAppWindow. Add test logic that calls the
component factory functions (mCommunity.component and mFirstParty.component) and
inspects the props that would be passed to SandboxedAppWindow to confirm that
each component factory correctly captures and passes through its respective
trust value ("community" for mCommunity and "first-party" for mFirstParty) to
the SandboxedAppWindow component.
| @pytest.mark.asyncio | ||
| async def test_public_install_endpoint_always_community(client): | ||
| r = await client.post( | ||
| "/api/userspace-apps/install", | ||
| files={"package": ("studio.taosapp", _zip(), "application/zip")}, | ||
| ) | ||
| assert r.status_code == 200, r.text | ||
| # Verify the stored record has community trust regardless of any manifest content. | ||
| rows = (await client.get("/api/userspace-apps")).json() | ||
| row = next((a for a in rows if a["app_id"] == "studio"), None) | ||
| assert row is not None | ||
| assert row["trust"] == "community" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add a regression test for public reinstall of an existing first-party app ID.
Current coverage validates only the fresh-install case. Add a case that seeds trust="first-party" and then calls public /install for the same app_id, asserting stored trust becomes community.
Suggested test shape
+@pytest.mark.asyncio
+async def test_public_reinstall_forces_existing_app_to_community(client, app):
+ store = app.state.userspace_apps
+ await store.install(
+ app_id="studio", name="Studio", version="1", app_type="web",
+ entry="index.html", icon="", permissions_requested=[], trust="first-party",
+ )
+
+ r = await client.post(
+ "/api/userspace-apps/install",
+ files={"package": ("studio.taosapp", _zip(), "application/zip")},
+ )
+ assert r.status_code == 200, r.text
+
+ row = await store.get("studio")
+ assert row is not None
+ assert row["trust"] == "community"🤖 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 `@tests/userspace/test_trust.py` around lines 118 - 130, Add a new test
function to validate that reinstalling an existing first-party app via the
public install endpoint downgrades its trust to community. The test should first
seed the database with an app having trust set to "first-party" (using the same
app_id "studio"), then call the public POST /api/userspace-apps/install endpoint
with the same package, and finally verify that the stored trust value is
downgraded to "community" rather than remaining first-party. This complements
the existing test_public_install_endpoint_always_community function which only
covers the fresh-install scenario.
…972 findings CRITICAL: the install UPSERT did not update trust on conflict, so a public install (always community) of an existing first-party app id would overwrite the bundle while keeping first-party privileges. Fixed two ways: - store.install UPSERT now sets trust=excluded.trust (a community reinstall downgrades trust; re-seeding first-party stays first-party) - the public install endpoint rejects (409) replacing an app already installed as first-party, so a trusted studio's bundle cannot be clobbered at all Also: SDK guards taosTheme against non-object/array payloads; regression tests for the 409 reject + the UPSERT trust update; SDK array-rejection test.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| status_code=501, | ||
| ) | ||
| existing = await store.get(manifest["id"]) | ||
| # A public install must never replace an app installed as first-party: that |
There was a problem hiding this comment.
CRITICAL: Public install extracts package before checking existing first-party trust
extract_package() writes files into apps_root/<manifest.id>/ before this guard runs. If a public install uses the same id as an existing first-party app, the bundle is overwritten even though the endpoint later returns 409; subsequent /bundle and /broker requests then use the clobbered files with trust='first-party'.
Move the trust/collision check before any write, or parse/validate the manifest without extracting before checking existing trust.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_public_install_endpoint_always_community(client): |
There was a problem hiding this comment.
WARNING: Public install trust test does not cover self-declared trust
This package uses WEB_MANIFEST without a trust field, so the test only proves the default install value. To cover the PR's security invariant that no manifest field can elevate trust, add a manifest variant with trust: first-party and assert the stored row still has trust == "community".
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "/api/userspace-apps/install", | ||
| files={"package": ("studio.taosapp", _zip(), "application/zip")}, | ||
| ) | ||
| assert r.status_code == 409 |
There was a problem hiding this comment.
CRITICAL: Reinstall rejection test does not verify bundle content is preserved
The assertion only checks DB trust remains first-party. Because extract_package() runs before the 409 guard, a colliding public install can still overwrite the first-party bundle and this test would pass. Assert the on-disk index.html content or a hash/mtime is unchanged after the rejected install.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
… complete (P3b #972 merged); P4 plan
1 participant
Phase P3b of the app system (#89): make the userspace app-runtime trust-aware so first-party packages (the studios; signature-verified later) run with a relaxed runtime while community packages stay tightly sandboxed.
Changes
trustcolumn (default 'community') via an idempotent ALTER-TABLE migration; install() gains atrustkwarg; get()/list() return it.trustthrough; inject the active theme tokens into the iframe on load + on theme change, for first-party apps only.Security boundary
Trust is never self-declared. The public POST /install hardcodes trust='community' regardless of manifest content. First-party is reachable only via an internal path (store.install(trust='first-party')), which is what P4 boot-seeding and P2 signature verification will use.
Notes
Rebased onto current dev, so it carries the set_permissions security fix that landed separately; one P3b test was updated so its community app requests the cap it grants (the secure contract). The first-party CSP is currently identical to community (separate constants for future divergence): the community CSP already permits what theming needs without weakening the sandbox, so the real first-party privilege is the broader capabilities + theme injection.
Tests
58 backend (tests/userspace/, incl. new test_trust.py) + 16 frontend pass; app smoke OK; tsc + build clean.
Part of #89.
Summary by CodeRabbit
window.taos.themeAPI (get/subscribe).