Repository navigation
fix(security): enforce declared permissions on every context facade, encrypt secret settings at rest, harden marketplace installs - #42
Merged
tajanthind merged 1 commit intoSep 4, 2026
Conversation
…ts at rest, harden marketplace installs
Security review — five fixes:
1. Manifest permissions now gate EVERY context facade, not just FSContext.
- PermissionChecker(plugin_id, permissions) shared by LibraryContext
(library:read / library:write), SettingsContext ("settings"),
FSContext (filesystem:downloads / filesystem:music), and
PluginContext.http ("network" — undeclared means http is None).
- Fail closed: a facade built without an explicit checker permits
nothing; core code uses core_context_checker() at its two call sites.
2. Marketplace install hardening:
- sha256 is now MANDATORY — an index entry without it is refused
(fail closed; previously it was optional).
- _safe_extract() resolves every archive member against the plugin
directory and refuses any member that would escape it (zip-slip).
3. Manifest "type": "secret" settings are encrypted at rest (Fernet;
key in CONFIG_DIR/plugin_secret.key, mode 0600, outside the DB):
- plugins/secret_store.py (encrypt/decrypt/looks_encrypted).
- PluginSetting.secret column + migration.
- SettingsContext and the settings API encrypt on write, decrypt on
read; boot backfill encrypts pre-existing plaintext secret rows
(idempotent).
4. PluginManager._loaded_for_instance is now an O(1) reverse index
id(instance) -> plugin_id instead of a linear scan per invoke_provider.
5. PluginManager lock switched from threading.Lock to cooperative
gevent.lock.RLock (threading fallback outside gevent contexts).
Bundled + example plugin manifests declare the permissions they actually
use (kept byte-identical with the fnack-plugins catalog); AUTHORING.md
documents the enforced permission contract, secret-at-rest, mandatory
sha256 / zip-slip rejection, and the direct-import limitation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security review — five fixes, one PR.
1. Manifest permissions gate EVERY context facade (was: only FSContext)
PermissionChecker(plugin_id, permissions)shared by all facades.LibraryContextnow requireslibrary:read(all read methods) /library:write(all write methods).SettingsContextrequiressettings.FSContextrequiresfilesystem:downloads/filesystem:music.PluginContext.httpisNoneunless the manifest declaresnetwork— undeclared outbound HTTP is impossible via the context.core_context_checker().2. Marketplace install hardening (
plugins/registry.py)sha256in the repo index is now mandatory — an entry without it is refused (previously optional)._safe_extract(): every archive member is resolved against the plugin dir; any member escaping it (zip-slip) raisesRegistryErrorbefore extraction.3. Secret settings encrypted at rest (
plugins/secret_store.py,models.py,api.py,context.py)settings_schema"type": "secret"fields are Fernet-encrypted before storage; key lives inCONFIG_DIR/plugin_secret.key(0600), outside the DB.PluginSetting.secretcolumn + migration; boot backfill encrypts pre-existing plaintext secret rows (idempotent — Fernet tokens skipped).context.settings/ settings API.4. O(1)
_loaded_for_instance(plugins/manager.py)id(instance) -> plugin_idmaintained under the manager lock; replaces the linear_plugins.values()scan on everyinvoke_provider().5. Cooperative lock (
plugins/manager.py)gevent.lock.RLock(threadingRLockfallback outside gevent) replacesthreading.Lock.Supporting changes
4f0bc4bwith release zips clobbered).docs/plugins/AUTHORING.mddocuments the enforced permission contract, secret-at-rest, mandatory sha256 / zip-slip rejection, and the honest limitation that a plugin importingrequestsdirectly bypassescontext.http.tests/architecture/test_plugin_permissions_and_secrets.py(permission gating, secret encryption at rest, sha256/zip-slip rejection).cryptography>=42.0added to requirements.Verification
fnack.acoustidfrom the marketplace, set itsapi_key→ DB row hassecret=1and a Fernet token (not plaintext); GET returns plaintext; survives container restart with no double-encryption.