Refactor connection handling in extractShellConnection to improve volatile ID management and ensure secure credential handling
This commit is contained in:
@@ -504,31 +504,40 @@ module.exports = {
|
||||
if (process.send && processArgs.processDisplayName === 'script') {
|
||||
const conn = await new Promise(resolve => {
|
||||
let resolved = false;
|
||||
const handler = message => {
|
||||
if (message?.msgtype === 'volatile-connection-response' && message.conid === conid) {
|
||||
process.removeListener('message', handler);
|
||||
clearTimeout(timeout);
|
||||
if (!resolved) {
|
||||
resolved = true;
|
||||
resolve(message.conn || null);
|
||||
}
|
||||
}
|
||||
};
|
||||
const timeout = setTimeout(() => {
|
||||
|
||||
const cleanup = () => {
|
||||
process.removeListener('message', handler);
|
||||
process.removeListener('disconnect', onDisconnect);
|
||||
clearTimeout(timeout);
|
||||
};
|
||||
|
||||
const settle = value => {
|
||||
if (!resolved) {
|
||||
resolved = true;
|
||||
resolve(null);
|
||||
cleanup();
|
||||
resolve(value);
|
||||
}
|
||||
}, 5000);
|
||||
};
|
||||
|
||||
const handler = message => {
|
||||
if (message?.msgtype === 'volatile-connection-response' && message.conid === conid) {
|
||||
settle(message.conn || null);
|
||||
}
|
||||
};
|
||||
|
||||
const onDisconnect = () => settle(null);
|
||||
|
||||
const timeout = setTimeout(() => settle(null), 5000);
|
||||
// Don't let the timer alone keep the process alive if all other work is done
|
||||
timeout.unref();
|
||||
|
||||
process.on('message', handler);
|
||||
process.once('disconnect', onDisconnect);
|
||||
|
||||
try {
|
||||
process.send({ msgtype: 'get-volatile-connection', conid });
|
||||
} catch {
|
||||
process.removeListener('message', handler);
|
||||
clearTimeout(timeout);
|
||||
resolved = true;
|
||||
resolve(null);
|
||||
settle(null);
|
||||
}
|
||||
});
|
||||
if (conn) {
|
||||
|
||||
@@ -38,12 +38,11 @@ function extractDriverApiParameters(values, direction, driver) {
|
||||
|
||||
export function extractShellConnection(connection, database) {
|
||||
const config = getCurrentConfig();
|
||||
const volatileId = getVolatileRemapping(connection._id);
|
||||
const hasVolatileMapping = volatileId !== connection._id;
|
||||
|
||||
// For volatile connections (ask-for-password), always use _id reference so
|
||||
// the backend can inject the password-bearing connection into the child process.
|
||||
if (hasVolatileMapping) {
|
||||
// Case 1: connection._id is the original ID and a volatile remap exists.
|
||||
// Use the volatile ID so the backend child process can look up the credentials.
|
||||
const volatileId = getVolatileRemapping(connection._id);
|
||||
if (volatileId !== connection._id) {
|
||||
return {
|
||||
_id: volatileId,
|
||||
engine: connection.engine,
|
||||
@@ -51,6 +50,19 @@ export function extractShellConnection(connection, database) {
|
||||
};
|
||||
}
|
||||
|
||||
// Case 2: apiCall.transformApiArgs already remapped the conid before the
|
||||
// connection was fetched, so connection._id IS already the volatile ID and
|
||||
// connection.unsaved === true. Falling through to allowShellConnection here
|
||||
// would embed plaintext credentials in the generated script — always use the
|
||||
// _id reference instead.
|
||||
if (connection.unsaved) {
|
||||
return {
|
||||
_id: connection._id,
|
||||
engine: connection.engine,
|
||||
database,
|
||||
};
|
||||
}
|
||||
|
||||
return config.allowShellConnection
|
||||
? {
|
||||
..._.omitBy(
|
||||
|
||||
Reference in New Issue
Block a user