Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins cannot read other plugin configs #88

Merged
merged 3 commits into from
Dec 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 8.0.0

- **Breaking change**: the config object now has the `config.global` namespace, and `config[pluginName]` namespace. A plugin can only access its own config, plus the global config. This is to prevent plugins from accessing each other's config, for preventive security and better code organization.

mixmix marked this conversation as resolved.
Show resolved Hide resolved
# 7.0.0

- **Breaking change**: Node.js >=16.0.0 is now required, due to the use of new JavaScript syntax
24 changes: 13 additions & 11 deletions PLUGINS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ like Secure-Scuttlebutt. It is highly extensible via plugins.
Plugins are simply NodeJS modules that export an `object` of form `{ name, version, manifest, init }`.

```js
// bluetooth-plugin.js
// bluetooth-plugin.js

module.exports = {
name: 'bluetooth',
Expand All @@ -20,10 +20,12 @@ module.exports = {
init: (api, opts) => {
// .. do things

// In opts, only opts.bluetooth and opts.global are available

// return things promised by the manifest:
return {
localPeers, // an async function (takes a callback)
updates // a function which returns a pull-stream source
updates // a function which returns a pull-stream source
}
}
}
Expand All @@ -37,20 +39,20 @@ method.

var SecretStack = require('secret-stack')

var App = SecretStack({ appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=' })
var App = SecretStack({ global: { appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=' } })
.use(require('./bluetooth-plugin'))

var app = App()
```

The plugin has now been mounted on the `secret-stack` instance and
methods exposed by the plugin can be accessed at `app.pluginName.methodName`
(e.g. `app.bluetooth.updates`
(e.g. `app.bluetooth.updates`)

---

Plugins can be used to for a number of different use cases, like adding
a persistent underlying database ([ssb-db](https://github.com/ssbc/ssb-db'))
a persistent underlying database ([ssb-db](https://github.com/ssbc/ssb-db'))
or layering indexes on top of the underlying store ([ssb-links](https://github.com/ssbc/ssb-links)).

It becomes very easy to lump a bunch of plugins together and create a
Expand All @@ -60,7 +62,7 @@ more sophisticated application.
var SecretStack = require('secret-stack')
var config = require('./some-config-file')

var Server = SecretStack({ appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=' })
var Server = SecretStack({ global: { appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=' } })
.use(require('ssb-db')) // added persistent log storage
.use(require('ssb-gossip')) // added peer gossip capabilities
.use(require('ssb-replicate')) // can now replicate other logs with peers
Expand All @@ -69,7 +71,7 @@ var Server = SecretStack({ appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=
var server = Server(config) // start application
```

## Plugin Format
## Plugin Format

A valid plugin is an `Object` of form `{ name, version, manifest, init }`

Expand Down Expand Up @@ -99,13 +101,13 @@ of plugins will be called in the order they were registered with `use`.

The `init` function of a plugin will be passed:
- `api` - _Object_ the secret-stack app so far
- `opts` - the merge of the default-config secret-stack factory (App) was created with and the config the app was initialised with (app).
- `opts` - configurations available to this plugin are `opts.global` and `opts[plugin.name]`
- `permissions` - _Object_ the permissions so far
- `manifest` - _Object_ the manifest so far

If `plugin.name` is a string, then the return value of init is mounted like `api[plugin.name] = plugin.init(api, opts)`

(If there's no `plugin.name` then the results of `init` are merged directly withe the `api` object!)
(If there's no `plugin.name` then the results of `init` are merged directly with the `api` object!)

Note, each method on the api gets wrapped with [hoox](https://github.com/dominictarr/hoox)
so that plugins may intercept that function.
Expand All @@ -124,7 +126,7 @@ Any permissions provided will be merged into the main permissions,
prefixed with the plugin name.

e.g. In this case we're giving anyone access to `api.bluetooth.localPeers`,
and the permission would be listed `'bluetooth.localPeers'`
and the permission would be listed `'bluetooth.localPeers'`

```js
module.exports = {
Expand All @@ -143,7 +145,7 @@ module.exports = {
// return things promised by the manifest:
return {
localPeers, // an async function (takes a callback)
updates // a function which returns a pull-stream source
updates // a function which returns a pull-stream source
}
}
}
Expand Down
11 changes: 6 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var databasePlugin = require('./some-database')
var bluetoothPlugin = require('./bluetooth')
var config = require('./some-config')

var App = SecretStack({ appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=' })
var App = SecretStack({ global: { appKey: '1KHLiKZvAvjbY1ziZEHMXawbCEIM6qwjCDm3VYRan/s=' } })
Copy link
Member

Choose a reason for hiding this comment

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

I have never used appKey. I would be in favoure of deprecating (wasn't it replaced with caps?)

I also don't think "global" is needed here? It feels more implicit to me that these are global?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have never used appKey. I would be in favoure of deprecating (wasn't it replaced with caps?)

Out of scope?

also don't think "global" is needed here? It feels more implicit to me that these are global?

It's necessary in case you want to specify at this point some plugin-specific config, then you need a way of saying that the other configs are global, not plugin-specific. It can also be generally less confusing, if both SecretStack({ ... }) and .create(null, { ... }) object shapes follow the same schema.

.use(databasePlugin)
.use(bluetoothPlugin)

Expand Down Expand Up @@ -57,11 +57,12 @@ Returns the App (with plugin now installed)

Start the app and returns an EventEmitter with methods (core and plugin) attached.

`config` is an (optional) Object with any properties:
- `keys` - _String_ a sodium ed25519 key pair
- ... - (optional)
`config` is an (optional) Object with:
- `config.global` - an object containing data available for all plugins
- `config.global.keys` - _String_ a sodium ed25519 key pair
- `config[pluginName]` - an object containing data only available to the plugin with name `pluginName`. Note that `pluginName` is the camelCase of `plugin.name`.

`config` will be passed to each plugin as they're initialised (as `merge(opts, config)` which opts were those options `SecretStack` factory was initialised with).
`config` will be passed to each plugin as they're initialised (as `merge(opts, config)` which opts were those options `SecretStack` factory was initialised with), with only `config.global` and `config[pluginName]` available to each plugin.

This `app` as an EventEmitter emits the following events:

Expand Down
76 changes: 49 additions & 27 deletions lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function merge (a, b, mapper) {
!(b[k] instanceof Uint8Array) &&
!Array.isArray(b[k])
) {
a[k] = {}
a[k] ??= {}
staltz marked this conversation as resolved.
Show resolved Hide resolved
merge(a[k], b[k], mapper)
} else {
a[k] = mapper(b[k], k)
mixmix marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -36,28 +36,47 @@ function merge (a, b, mapper) {
return a
}

/**
* @param {Record<string, any>} fullConfig
* @param {{name?: string}} plugin
*/
function buildPluginConfig (fullConfig, plugin) {
if (plugin.name) {
const camelCaseName = /** @type {string} */ (u.toCamelCase(plugin.name))
staltz marked this conversation as resolved.
Show resolved Hide resolved
return {
[camelCaseName]: fullConfig[camelCaseName],
global: fullConfig.global ?? {}
}
} else {
return {
global: fullConfig.global ?? {}
}
}
}

/**
* @param {Array<any>} plugins
* @param {any} defaultConfig
*/
function Api (plugins, defaultConfig) {
/**
* @param {any} inputOpts
* @param {any} inputConfig
*/
function create (inputOpts) {
const opts = merge(merge({}, defaultConfig), inputOpts)
function create (inputConfig) {
const config = merge(merge({}, defaultConfig), inputConfig)
// change event emitter to something with more rigorous security?
let api = new EventEmitter()
create.plugins.forEach((plug) => {
let _api = plug.init.call(
for (const plugin of create.plugins) {
const pluginConfig = buildPluginConfig(config, plugin)
let _api = plugin.init.call(
{},
api,
opts,
pluginConfig,
create.permissions,
create.manifest
)
if (plug.name) {
const camelCaseName = u.toCamelCase(plug.name)
if (plugin.name) {
const camelCaseName = u.toCamelCase(plugin.name)
if (camelCaseName) {
/** @type {Record<string, unknown>} */
const o = {}
Expand All @@ -75,14 +94,14 @@ function Api (plugins, defaultConfig) {
(val, key) => {
if (typeof val === 'function') {
val = Hookable(val)
if (plug.manifest && plug.manifest[key] === 'sync') {
if (plugin.manifest && plugin.manifest[key] === 'sync') {
u.hookOptionalCB(val)
}
}
return val
}
)
})
}
return api
}

Expand All @@ -92,48 +111,51 @@ function Api (plugins, defaultConfig) {

create.use =
/**
* @param {any} plug
* @param {any} plugin
*/
function use (plug) {
if (Array.isArray(plug)) {
plug.forEach(create.use)
function use (plugin) {
if (Array.isArray(plugin)) {
plugin.forEach(create.use)
return create
}

if (!plug.init) {
if (typeof plug === 'function') {
create.plugins.push({ init: plug })
if (!plugin.init) {
if (typeof plugin === 'function') {
create.plugins.push({ init: plugin })
return create
} else {
throw new Error('plugins *must* have "init" method')
}
}

if (plug.name && typeof plug.name === 'string') {
const found = create.plugins.some((p) => p.name === plug.name)
if (plugin.name && typeof plugin.name === 'string') {
if (plugin.name === 'global') {
throw new Error('plugin named "global" is reserved')
}
const found = create.plugins.some((p) => p.name === plugin.name)
if (found) {
// prettier-ignore
console.error('plugin named:' + plug.name + ' is already loaded, skipping')
console.error('plugin named:' + plugin.name + ' is already loaded, skipping')
return create
}
}

const name = plug.name
if (plug.manifest) {
const name = plugin.name
if (plugin.manifest) {
create.manifest = u.merge.manifest(
create.manifest,
plug.manifest,
plugin.manifest,
u.toCamelCase(name)
)
}
if (plug.permissions) {
if (plugin.permissions) {
create.permissions = u.merge.permissions(
create.permissions,
plug.permissions,
plugin.permissions,
u.toCamelCase(name)
)
}
create.plugins.push(plug)
create.plugins.push(plugin)

return create
}
Expand Down
42 changes: 21 additions & 21 deletions lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,26 +151,26 @@ module.exports = {
init (api, opts, permissions, manifest) {
/** @type {number} */
let timeoutInactivity
if (opts.timers?.inactivity && u.isNumber(opts.timers?.inactivity)) {
timeoutInactivity = opts.timers?.inactivity
if (u.isNumber(opts.global.timers?.inactivity)) {
timeoutInactivity = /** @type {number} */ (opts.global.timers?.inactivity)
}
// if opts.timers are set, pick a longer default
// but if not, set a short default (as needed in the tests)
timeoutInactivity ??= opts.timers ? 600e3 : 5e3
timeoutInactivity ??= opts.global.timers ? 600e3 : 5e3

if (!opts.connections) {
if (!opts.global.connections) {
/** @type {Incoming} */
const netIn = {
scope: ['device', 'local', 'public'],
transform: 'shs',
...(opts.host ? { host: opts.host } : null),
...(opts.port ? { port: opts.port } : null)
...(opts.global.host ? { host: opts.global.host } : null),
...(opts.global.port ? { port: opts.global.port } : null)
}
/** @type {Outgoing} */
const netOut = {
transform: 'shs'
}
opts.connections = {
opts.global.connections = {
incoming: {
net: [netIn]
},
Expand Down Expand Up @@ -208,10 +208,10 @@ module.exports = {
/** @type {Array<[unknown, unknown]>} */
const clientSuites = []

for (const incTransport in opts.connections?.incoming) {
opts.connections.incoming[incTransport].forEach((inc) => {
transforms.forEach((transform) => {
transports.forEach((transport) => {
for (const incTransport in opts.global.connections?.incoming) {
for (const inc of opts.global.connections.incoming[incTransport]) {
for (const transform of transforms) {
for (const transport of transports) {
if (
transport.name === incTransport &&
transform.name === inc.transform
Expand All @@ -226,15 +226,15 @@ module.exports = {
debug('creating server %s %s host=%s port=%d scope=%s', incTransport, transform.name, inc.host, inc.port, inc.scope ?? 'undefined')
serverSuites.push([msPlugin, msTransformPlugin])
}
})
})
})
}
}
}
}

for (const outTransport in opts.connections?.outgoing) {
opts.connections.outgoing[outTransport].forEach((out) => {
transforms.forEach((transform) => {
transports.forEach((transport) => {
for (const outTransport in opts.global.connections?.outgoing) {
for (const out of opts.global.connections.outgoing[outTransport]) {
for (const transform of transforms) {
for (const transport of transports) {
if (
transport.name === outTransport &&
transform.name === out.transform
Expand All @@ -243,9 +243,9 @@ module.exports = {
const msTransformPlugin = transform.create()
clientSuites.push([msPlugin, msTransformPlugin])
}
})
})
})
}
}
}
}

msClient = MultiServer(clientSuites)
Expand Down
Loading
Loading