Skip to content

fix: harden stored plugin trust and startup execution path#6719

Open
Chaitu7032 wants to merge 1 commit intosugarlabs:masterfrom
Chaitu7032:plugin-fix
Open

fix: harden stored plugin trust and startup execution path#6719
Chaitu7032 wants to merge 1 commit intosugarlabs:masterfrom
Chaitu7032:plugin-fix

Conversation

@Chaitu7032
Copy link
Copy Markdown
Contributor

@Chaitu7032 Chaitu7032 commented Apr 19, 2026

Summary

This PR fixes a critical client-side security issue in plugin loading.

Before this change, plugins stored in browser storage could be treated as trusted and auto-executed on startup, which created a persistent execution path if storage was poisoned once.

Now, stored plugins are no longer blindly trusted, approval is explicit, and trust is scoped safely so malicious persistence cannot silently re-authorize itself across sessions.

Relevant areas:

  • utils.js
  • activity.js

What this change does

  • Stops auto-trusting storage source by default.
  • Adds explicit approval flow for stored plugin payloads using a stable plugin signature (hash/fingerprint).
  • Introduces an allowlist check before treating stored plugin code as execution-trusted.
  • Adds plugin safe mode flags for failure paths (for example parse failure), so startup can skip risky stored plugin execution.
  • Keeps built-in plugin path trusted, so normal built-in plugin UX still works.
  • Moves trust allowlist storage to session scope, not persistent local scope, so poisoned local storage cannot persistently self- authorize across browser restarts.

Behavior after this PR

  • Built-in plugins still load normally (no regression in normal built-in flow).
  • Stored plugins are gated by approval logic and trust checks.
  • If stored plugin data is invalid or declined, it does not execute.
  • Trust is not persistently self-recoverable from poisoned local storage because allowlist trust is session-scoped.

Tests added and updated

Added targeted tests for new trust logic in:

  • utils.test.js

New test coverage includes:

  • Deterministic plugin signature generation.
  • Trust check is false before approval and true after approval.
  • Safe mode state and reason are set/cleared correctly.
  • Session-scoped trust storage behavior.

ScreenShots

jest tests

image

focused test

image

PR Category

  • Bug Fix

@github-actions
Copy link
Copy Markdown
Contributor

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 42.4% | Branches: 35.1% | Functions: 46.74% | Lines: 42.81%

@github-actions github-actions bot added bug fix Fixes a bug or incorrect behavior size/L Large: 250-499 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels Apr 19, 2026
Copy link
Copy Markdown
Contributor

@Ashutoshx7 Ashutoshx7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is clean pr for the plugin persistence
@walterbender l

@walterbender
Copy link
Copy Markdown
Member

have you tried loading a plugin? It may be a preexisting regression, but while the Maths plugin loads, none of its block run.

@Ashutoshx7
Copy link
Copy Markdown
Contributor

Ashutoshx7 commented Apr 20, 2026

hy i can reproduce what walter said , the regression is in the seassion scoped trust storage when u load maths with the built-in flow, prepepare plugin exorts save the hash to sessionstorage so on reload the sesssion storage is cleared so the hash is gone then then confirm dialog appears and if dismissed , executiontrusted stays false so the palette render but blocks do nothing

the fix would be to store the approval hash in localstorage( not session sorage ) for plugins that came thorugh explicit build in load flow and only use session-scropte for unknow/untrusted source

@Chaitu7032
Copy link
Copy Markdown
Contributor Author

Chaitu7032 commented Apr 20, 2026

Thanks for verifying it walter sir . I will test it once again on my side and update you with the conclusions shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior size/L Large: 250-499 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants