Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00
Code Issues Activity

Added labs setting input validation

Browse source 
refs https://github.com/TryGhost/Team/issues/757

- To safeguard from mise of a very permissing "object" value of the "labs" setting this change introduces an "allowlist" approach to filtering unrecognized labs flags
- Should allow maintainers to have a clear view of which labs flags are currently in use and manage them accordingly
This commit is contained in:
Naz 2021-06-04 19:56:16 +04:00 • committed by naz
parent 8ab43b84d5
commit cd35358fdb
8 changed files with 75 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
core/server/api/canary/utils/serializers/input/settings.js
View file

@ -2,6 +2,7 @@ const _ = require('lodash');
const url = require('./utils/url');
const typeGroupMapper = require('../../../../shared/serializers/input/utils/settings-filter-type-group-mapper');
const settingsCache = require('../../../../../services/settings/cache');
const {WRITABLE_KEYS_ALLOWLIST} = require('../../../../../services/labs');
const DEPRECATED_SETTINGS = [
'bulk_email_settings',
@ -158,6 +159,19 @@ module.exports = {
setting.key = 'lang';
}
if (setting.key === 'labs') {
const inputLabsValue = JSON.parse(setting.value);
const filteredLabsValue = {};
for (const flag in inputLabsValue) {
if (WRITABLE_KEYS_ALLOWLIST.includes(flag)) {
filteredLabsValue[flag] = inputLabsValue[flag];
}
}
setting.value = JSON.stringify(filteredLabsValue);
}
setting = url.forSetting(setting);
});

14
core/server/api/v2/utils/serializers/input/settings.js
View file

@ -2,6 +2,7 @@ const _ = require('lodash');
const url = require('./utils/url');
const typeGroupMapper = require('../../../../shared/serializers/input/utils/settings-filter-type-group-mapper');
const settingsCache = require('../../../../../services/settings/cache');
const {WRITABLE_KEYS_ALLOWLIST} = require('../../../../../services/labs');
const DEPRECATED_SETTINGS = [
'bulk_email_settings',
@ -138,6 +139,19 @@ module.exports = {
setting.value = JSON.parse(setting.value).isActive;
}
if (setting.key === 'labs') {
const inputLabsValue = JSON.parse(setting.value);
const filteredLabsValue = {};
for (const value in inputLabsValue) {
if (WRITABLE_KEYS_ALLOWLIST.includes(value)) {
filteredLabsValue[value] = inputLabsValue[value];
}
}
setting.value = JSON.stringify(filteredLabsValue);
}
setting = url.forSetting(setting);
});

14
core/server/api/v3/utils/serializers/input/settings.js
View file

@ -2,6 +2,7 @@ const _ = require('lodash');
const url = require('./utils/url');
const typeGroupMapper = require('../../../../shared/serializers/input/utils/settings-filter-type-group-mapper');
const settingsCache = require('../../../../../services/settings/cache');
const {WRITABLE_KEYS_ALLOWLIST} = require('../../../../../services/labs');
const DEPRECATED_SETTINGS = [
'bulk_email_settings',
@ -154,6 +155,19 @@ module.exports = {
setting.value = JSON.parse(setting.value).isActive;
}
if (setting.key === 'labs') {
const inputLabsValue = JSON.parse(setting.value);
const filteredLabsValue = {};
for (const value in inputLabsValue) {
if (WRITABLE_KEYS_ALLOWLIST.includes(value)) {
filteredLabsValue[value] = inputLabsValue[value];
}
}
setting.value = JSON.stringify(filteredLabsValue);
}
setting = url.forSetting(setting);
});

13
core/server/models/settings.js
View file

@ -9,6 +9,8 @@ const i18n = require('../../shared/i18n');
const errors = require('@tryghost/errors');
const validation = require('../data/validation');
const urlUtils = require('../../shared/url-utils');
const {WRITABLE_KEYS_ALLOWLIST} = require('../services/labs');
const internalContext = {context: {internal: true}};
let Settings;
let defaultSettings;
@ -341,6 +343,17 @@ Settings = ghostBookshelf.Model.extend({
throw new errors.ValidationError(validationErrors.join('\n'));
}
},
async labs(model) {
const flags = JSON.parse(model.get('value'));
for (const flag in flags) {
if (!WRITABLE_KEYS_ALLOWLIST.includes(flag)) {
throw new errors.ValidationError({
message: `Settings lab value cannot have value other then ${WRITABLE_KEYS_ALLOWLIST.join(', ')}`
});
}
}
},
async stripe_plans(model, options) {
const plans = JSON.parse(model.get('value'));
for (const plan of plans) {

8
core/server/services/labs.js
View file

@ -6,6 +6,14 @@ const i18n = require('../../shared/i18n');
const logging = require('../../shared/logging');
const settingsCache = require('../services/settings/cache');
// NOTE: this allowlist is meant to be used to filter out any unexpected
// input for the "labs" setting value
const WRITABLE_KEYS_ALLOWLIST = [
'activitypub'
];
module.exports.WRITABLE_KEYS_ALLOWLIST = WRITABLE_KEYS_ALLOWLIST;
module.exports.getAll = () => ({
members: settingsCache.get('members_signup_access') !== 'none'
});

7
test/regression/api/canary/admin/settings_spec.js
View file

@ -653,12 +653,13 @@ describe('Settings API (canary)', function () {
});
});
it('Can edit labs', async function () {
it('Can edit only allowed labs keys', async function () {
const settingToChange = {
settings: [{
key: 'labs',
value: JSON.stringify({
matchHelper: true
activitypub: true,
gibberish: true
})
}]
};
@ -680,7 +681,7 @@ describe('Settings API (canary)', function () {
jsonResponse.settings[0].key.should.eql('labs');
jsonResponse.settings[0].value.should.eql(JSON.stringify({
matchHelper: true
activitypub: true
}));
});

7
test/regression/api/v2/admin/settings_spec.js
View file

@ -521,12 +521,13 @@ describe('Settings API (v2)', function () {
});
});
it('Can edit labs', async function () {
it('Can edit only allowed labs keys', async function () {
const settingToChange = {
settings: [{
key: 'labs',
value: JSON.stringify({
matchHelper: true
activitypub: true,
gibberish: true
})
}]
};
@ -548,7 +549,7 @@ describe('Settings API (v2)', function () {
jsonResponse.settings[0].key.should.eql('labs');
jsonResponse.settings[0].value.should.eql(JSON.stringify({
matchHelper: true
activitypub: true
}));
});

7
test/regression/api/v3/admin/settings_spec.js
View file

@ -464,12 +464,13 @@ describe('Settings API (v3)', function () {
});
});
it('Can edit labs', async function () {
it('Can edit only allowed labs keys', async function () {
const settingToChange = {
settings: [{
key: 'labs',
value: JSON.stringify({
matchHelper: true
activitypub: true,
gibberish: true
})
}]
};
@ -491,7 +492,7 @@ describe('Settings API (v3)', function () {
jsonResponse.settings[0].key.should.eql('labs');
jsonResponse.settings[0].value.should.eql(JSON.stringify({
matchHelper: true
activitypub: true
}));
});