Skip to content
Open
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
25 changes: 23 additions & 2 deletions server/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,13 @@ function makeApp(authAddress, cdapConfig, uiSettings) {
* be used and we need to persist this information somewhere.
*/
app.post('/updateTheme', function (req, res) {
// This endpoint is only for testing themes in development and must not be
// active in production (it has no persistent effect and opens unnecessary
// attack surface). Return 401 so callers get a clear signal rather than a
// silent no-op.
if (isModeProduction()) {
return res.status(401).send('This endpoint is not available in production');
}
let authToken = req.headers.authorization;
if (
!req.headers['session-token'] ||
Expand All @@ -717,10 +724,24 @@ function makeApp(authAddress, cdapConfig, uiSettings) {
) {
return res.status(500).send('Unable to validate session');
}
const uiThemePath = req.body.uiThemePath;
if (!uiThemePath) {
const requestedTheme = req.body.uiThemePath;
if (typeof requestedTheme !== 'string' || !requestedTheme) {
return res.status(500).send('UnKnown theme file. Please make sure the path is valid');
}
// This endpoint exists to switch between the theme files shipped in
// config/themes/ for testing (see doc comment above), not to load an
// arbitrary path. Resolve to a filename within that directory only,
// discarding any directory component the client supplies, so this can't
// be used to point at a file elsewhere on the filesystem. path.basename
// returns '..' or '.' unchanged for those exact inputs (it only strips
// directory components, it doesn't resolve '..' segments), so those two
// values need an explicit reject or they'd resolve one level above
// config/themes/.
const baseTheme = path.basename(requestedTheme);
if (baseTheme === '..' || baseTheme === '.' || !baseTheme) {
return res.status(500).send('UnKnown theme file. Please make sure the path is valid');
}
const uiThemePath = path.join(__dirname, 'config', 'themes', baseTheme);
try {
uiThemeConfig = uiThemeWrapper.extractUITheme(cdapConfig, uiThemePath);
} catch (e) {
Expand Down
21 changes: 16 additions & 5 deletions server/token.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
*/

import crypto from 'crypto';
import fs from 'fs';
import path from 'path';

/* global process, __dirname */

const EncryptionConstants = {
ONE_HOUR_MILLIS: 60 * 60 * 1000,
/**
Expand Down Expand Up @@ -97,15 +100,23 @@ function decrypt(encText, key) {
}

function getSecretFromCDAPConfig(cdapConfig, logger) {
let secretKey =
cdapConfig['session.secret.key'] ||
path.resolve(__dirname, 'config', 'development', 'session.secret.key');
Comment thread
herdiyana256 marked this conversation as resolved.
if (!logger) {
logger = console;
}
const secretKey = cdapConfig['session.secret.key'];
if (!secretKey) {
logger.warn(
'Secret key missing. This is required to generate a strong time-based token to prevent cswh'
// In development, fall back to reading the actual secret file on disk.
// This avoids the old bug where the file *path string* was used as key
// material instead of the file's contents.
if (process.env.NODE_ENV === 'development') {
const keyFilePath = path.resolve(__dirname, 'config', 'development', 'session_secret.key');
return fs.readFileSync(keyFilePath, 'utf8');
}
// In production (or any other environment), fail closed: a deployment
// must explicitly configure a real secret via 'session.secret.key'.
throw new Error(
"'session.secret.key' is not configured. A real secret is required to generate " +
'or validate session tokens.'
);
}
return secretKey;
Expand Down
25 changes: 25 additions & 0 deletions server/token_test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,30 @@ function testMismatch() {
assert(!isTokenValid);
}

// A missing 'session.secret.key' must not fall back to any predictable value.
// generateToken should refuse to issue a token, and validateToken should
// reject everything (including a token forged with the old, predictable
// fallback), rather than accepting a guessable secret.
function testMissingSecretKeyRefusesToken() {
const cdapConfigNoSecret = { 'instance.metadata.id': 'test-instance' };

let threw = false;
try {
generateToken(cdapConfigNoSecret, console);
} catch (e) {
threw = true;
}
assert(threw, 'generateToken should throw when session.secret.key is not configured');

const isValid = validateToken('anything-at-all', cdapConfigNoSecret, console);
assert(!isValid, 'validateToken should reject when session.secret.key is not configured');

console.log('testMissingSecretKeyRefusesToken passed');
}

testMatch();
testMissingSecretKeyRefusesToken();
// Note: testMismatch() fails on master independent of this change, since
// authToken isn't actually part of the token or the comparison in either
// generateToken or validateToken. Left as-is, out of scope here.
testMismatch();
22 changes: 12 additions & 10 deletions server/uiThemeWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ function extractUIFeaturesFromConfig(cdapConfig) {
return featuresMap;
}

// Theme files are plain JSON data, not code. Read and parse them directly
// instead of require()'ing them to avoid executing attacker-controlled input.
function readThemeJSON(themePath) {
return JSON.parse(fs.readFileSync(themePath, 'utf8'));
}

function mergeUIThemeWithConfig(cdapConfig, themeConfig) {
const configFeatures = {
features: extractUIFeaturesFromConfig(cdapConfig),
Expand All @@ -71,11 +77,9 @@ export function extractUITheme(cdapConfig, uiThemePath) {
// Absolute path
if (uiThemePath[0] === '/') {
try {
if (__non_webpack_require__.resolve(uiThemePath)) {
uiThemeConfig = __non_webpack_require__(uiThemePath);
log.info(`UI using theme file: ${uiThemePath}`);
return mergeUIThemeWithConfig(cdapConfig, uiThemeConfig);
}
uiThemeConfig = readThemeJSON(uiThemePath);
log.info(`UI using theme file: ${uiThemePath}`);
return mergeUIThemeWithConfig(cdapConfig, uiThemeConfig);
} catch (e) {
log.info('UI Theme file not found at: ', uiThemePath);
throw e;
Expand Down Expand Up @@ -105,11 +109,9 @@ export function extractUITheme(cdapConfig, uiThemePath) {
}
themePath = path.join(__dirname, themePath);

if (__non_webpack_require__.resolve(themePath)) {
uiThemeConfig = __non_webpack_require__(themePath);
log.info(`UI using theme file: ${themePath}`);
return mergeUIThemeWithConfig(cdapConfig, uiThemeConfig);
}
uiThemeConfig = readThemeJSON(themePath);
log.info(`UI using theme file: ${themePath}`);
return mergeUIThemeWithConfig(cdapConfig, uiThemeConfig);
} catch (e) {
// This will show the user what the full path is.
// This should help them give proper relative path
Expand Down
51 changes: 51 additions & 0 deletions server/uiThemeWrapper.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright © 2026 Cask Data, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

import fs from 'fs';
import os from 'os';
import path from 'path';
import { extractUITheme } from 'server/uiThemeWrapper';

describe('extractUITheme', () => {
const cdapConfig = { 'ui.theme.file': true };

test('loads a plain JSON theme file', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ui-theme-test-'));
const themePath = path.join(dir, 'theme.json');
fs.writeFileSync(themePath, JSON.stringify({ content: { theme: 'ok' } }));

const result = extractUITheme(cdapConfig, themePath);
expect(result.content.theme).toBe('ok');
});

// Regression test: extractUITheme used to load the theme file with
// __non_webpack_require__, which executes .js files as Node modules
// instead of just reading them as data. A path ending in .js that runs
// code (instead of failing to parse as JSON) means this is still
// executing the file rather than reading it.
test('does not execute a .js file passed as the theme path', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ui-theme-test-'));
const canaryPath = path.join(dir, 'canary.txt');
const themePath = path.join(dir, 'theme.js');
fs.writeFileSync(
themePath,
`require('fs').writeFileSync(${JSON.stringify(canaryPath)}, 'executed');\nmodule.exports = { content: { theme: 'pwned' } };\n`
);

expect(() => extractUITheme(cdapConfig, themePath)).toThrow();
expect(fs.existsSync(canaryPath)).toBe(false);
});
});
Loading