mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-27 18:57:09 +00:00
feat: add target-aware RBAC authorization to Scheduled Operations (#1745)
* feat: add target-aware RBAC authorization to Scheduled Operations Replace the blanket requireAdmin gate on all 9 scheduled-tasks endpoints with per-action permission checks derived from the centralized action registry. Each scheduled action now declares the existing permission it requires: stack lifecycle actions need stack:deploy on the target stack, node-wide operations need node:manage on the target node, prune stays admin-only via system:settings, and snapshot requires unscoped node:manage. Key changes: - Backend registry: add permission field and resolveTaskPermissionScope - Routes: replace requireAdmin with requireTaskPermission, filter GET listing by permission, add two-phase PUT check - Scheduler: revalidate creator permission at execution time, auto-disable on revocation (TaskAuthorizationError) - Database: new creator_user_id column with migration and backfill - Frontend: canScheduleAction/canScheduleAny helpers, reachability gate via scheduledOpsAccessible, stack context menu uses canDeploy not isAdmin, permissions field on all ScheduledActionDefinitions - Expose checkPermissionForSubject for in-process callers (scheduler) No new PermissionAction values are added. Uses the existing stack:deploy, node:manage, and system:settings matrix. Scoped Admiral grants work on the exact (nodeId, stackName) target per SEN-438. * test: add RBAC coverage for scheduled operations authorization * test: update frontend tests for scheduled-ops RBAC gate changes - useStackMenuItems: gate Schedule task on canDeploy not isAdmin; add test for canDeploy=true, non-admin case - buildNavigationModel: default scheduledOpsAccessible to true (default ctx is admin, who can always schedule) - useViewNavigationState: add stack:deploy to admin can() mocks so canScheduleAny resolves correctly * fix: keep checkPermission unchanged, add checkPermissionForSubject standalone The earlier refactoring that made checkPermission delegate to checkPermissionForSubject changed the call order of effectiveTier(req) relative to the admin bypass, which subtly broke the Community tier clamping in the audit-log route. Keep checkPermission byte-identical to the original and expose checkPermissionForSubject as a standalone function used only by the scheduler revalidation path. * fix: prefix unused selectorType parameter in resolveTaskPermissionScope * fix: address audit findings — existence oracle, revalidation test, action filtering Three corrections from the independent PR audit: RC-1 (action filtering): Wire canScheduleActionAnywhere into the action picker in ScheduledOperationsView so actions the user can never schedule (prune without system:settings, node:manage actions without a scoped grant) are filtered from the picker entirely. Add canScheduleAction check on the Create button against the currently selected target, so the submit button is disabled when the caller cannot schedule the chosen action on the chosen target. RC-2 (existence oracle): Six by-ID endpoints (GET /:id, DELETE /:id, PATCH /:id/toggle, POST /:id/run, GET /:id/runs/export, GET /:id/runs) now return a uniform 404 when permission is denied on an existing task, so an unauthorized caller cannot distinguish "task does not exist" from "task exists but you are not authorized." Added requireTaskExistsPermission helper for the 404 variant; POST create and PUT merged-scope checks keep requireTaskPermission (403). RC-3 (revalidation test): Fixed the orphan-creator test to assert the post-execution task state (auto-disabled, explicit error message) rather than wrapping executeTask in a try/catch with a no-op else branch, since executeTask catches TaskAuthorizationError internally and returns normally. Added canScheduleActionAnywhere helper and AuthContext mock to ScheduledOperationsView tests (38/38 pass). * fix: remove unused TaskAuthorizationError import from test * fix: use 404 on PUT phase-1 unauthorized-access check The PUT two-phase check's first phase (ownership verification) now uses requireTaskExistsPermission (404) instead of a manual 403, consistent with the six other by-ID endpoints. This prevents a caller from probing task-ID existence via the PUT route. * fix: address QA findings — reorder prechecks, surface permission reason Two live-confirmed fixes from the 3-node fleet QA pass: Finding #4 (offline-node error ordering): Swap the order of the node- reachability precheck and the creator-permission revalidation in SchedulerService.executeTask. Authorization now runs first, so a revoked grant is always surfaced as an explicit auto-disable with a clear error message, even when the target node is offline. Previously the reachability check ran first, hiding the revocation behind a misleading "target node is offline" error and leaving the task enabled indefinitely while the node was down. Finding #7 (unexplained Save dead-end): When the Create/Update button is disabled because canScheduleAction denies the selected action-target combination, a muted text line now appears below the button: "You do not have permission to schedule this action on the selected target." This gives scoped users meaningful feedback instead of a silently disabled button with no explanation. * chore: remove redundant !!formName guards The earlier short-circuit conditions in isSaveDisabled and saveDisabledReason already guarantee formName is truthy by the time the canSaveWithCurrentTarget check runs. The !!formName guard is a no-op — flagged by GitHub code-quality as 'Useless conditional: This negation always evaluates to true.'
This commit is contained in:
Anso
@@ -707,6 +707,8 @@ export interface ScheduledTask {
|
||||
cron_expression: string;
|
||||
enabled: number;
|
||||
created_by: string;
|
||||
/** The user ID who created this schedule. Null for legacy rows (pre-RBAC) where username resolution failed at migration time. */
|
||||
creator_user_id: number | null;
|
||||
created_at: number;
|
||||
updated_at: number;
|
||||
last_run_at: number | null;
|
||||
@@ -1946,6 +1948,18 @@ export class DatabaseService {
|
||||
maybeAddCol('scheduled_tasks', 'selector_value', 'TEXT DEFAULT NULL');
|
||||
maybeAddCol('scheduled_tasks', 'delete_after_run', 'INTEGER DEFAULT 0');
|
||||
maybeAddCol('scheduled_tasks', 'run_at', 'INTEGER DEFAULT NULL');
|
||||
maybeAddCol('scheduled_tasks', 'creator_user_id', 'INTEGER DEFAULT NULL');
|
||||
|
||||
// Backfill creator_user_id from the created_by username column.
|
||||
// Rows whose username no longer matches a user stay NULL (legacy,
|
||||
// unrevalidated path — they were created under the old requireAdmin gate).
|
||||
this.db.exec(`
|
||||
UPDATE scheduled_tasks
|
||||
SET creator_user_id = (
|
||||
SELECT id FROM users WHERE username = scheduled_tasks.created_by
|
||||
)
|
||||
WHERE creator_user_id IS NULL
|
||||
`);
|
||||
|
||||
// Recreate stack_update_status with composite PK (node_id, stack_name).
|
||||
// Original table had stack_name as sole PK which breaks when multiple nodes share stack names.
|
||||
@@ -6185,12 +6199,13 @@ export class DatabaseService {
|
||||
return this.db.prepare('SELECT * FROM scheduled_tasks WHERE id = ?').get(id) as ScheduledTask | undefined;
|
||||
}
|
||||
|
||||
public createScheduledTask(task: Omit<ScheduledTask, 'id'>): number {
|
||||
public createScheduledTask(task: Omit<ScheduledTask, 'id' | 'creator_user_id'> & { creator_user_id?: number | null }): number {
|
||||
const result = this.db.prepare(
|
||||
'INSERT INTO scheduled_tasks (name, target_type, target_id, node_id, action, cron_expression, enabled, created_by, created_at, updated_at, last_run_at, next_run_at, last_status, last_error, prune_targets, target_services, prune_label_filter, selector_type, selector_value, delete_after_run, run_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'
|
||||
'INSERT INTO scheduled_tasks (name, target_type, target_id, node_id, action, cron_expression, enabled, created_by, creator_user_id, created_at, updated_at, last_run_at, next_run_at, last_status, last_error, prune_targets, target_services, prune_label_filter, selector_type, selector_value, delete_after_run, run_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'
|
||||
).run(
|
||||
task.name, task.target_type, task.target_id, task.node_id,
|
||||
task.action, task.cron_expression, task.enabled, task.created_by,
|
||||
task.creator_user_id ?? null,
|
||||
task.created_at, task.updated_at, task.last_run_at, task.next_run_at,
|
||||
task.last_status, task.last_error, task.prune_targets, task.target_services,
|
||||
task.prune_label_filter, task.selector_type ?? null, task.selector_value ?? null,
|
||||
|
||||
@@ -2,6 +2,7 @@ import { CronExpressionParser } from 'cron-parser';
|
||||
import { DatabaseService } from './DatabaseService';
|
||||
import type { ScheduledTask } from './DatabaseService';
|
||||
import { LicenseService } from './LicenseService';
|
||||
import type { LicenseTier } from './license-types';
|
||||
import { PROXY_TIER_HEADER, deployProvenanceHeaders } from './license-headers';
|
||||
import DockerController from './DockerController';
|
||||
import { ComposeService } from './ComposeService';
|
||||
@@ -35,6 +36,8 @@ import { filterContainersByComposeService } from '../helpers/composeServiceMatch
|
||||
import { excludeSelfContainers } from '../helpers/excludeSelfContainers';
|
||||
import { enforcePolicyPreDeploy } from './PolicyEnforcement';
|
||||
import { summarizeBlockReasons } from '../utils/policy-risk';
|
||||
import { resolveTaskPermissionScope, type BackendScheduledAction, type TargetType } from './scheduledActionRegistry';
|
||||
import { checkPermissionForSubject } from '../middleware/permissions';
|
||||
|
||||
const TRIVY_UPDATE_CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000;
|
||||
const TRIVY_UPDATE_CHECK_STARTUP_DELAY_MS = 5 * 60 * 1000;
|
||||
@@ -42,6 +45,14 @@ const TRIVY_UPDATE_CHECK_STARTUP_DELAY_MS = 5 * 60 * 1000;
|
||||
const TRIVY_REDETECT_INTERVAL_MS = 10 * 60 * 1000;
|
||||
const STALE_SCAN_THRESHOLD_MS = 15 * 60 * 1000;
|
||||
|
||||
/** Thrown when a scheduled task's creator no longer has permission for the target action. */
|
||||
export class TaskAuthorizationError extends Error {
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = 'TaskAuthorizationError';
|
||||
}
|
||||
}
|
||||
|
||||
export class SchedulerService {
|
||||
private static instance: SchedulerService;
|
||||
private intervalId: ReturnType<typeof setInterval> | null = null;
|
||||
@@ -303,6 +314,38 @@ export class SchedulerService {
|
||||
});
|
||||
|
||||
try {
|
||||
// Permission revalidation: for automatic runs, verify the creator still holds
|
||||
// the required permission. Runs before the node-reachability check so that
|
||||
// a revoked authorization is surfaced even when the target node is offline
|
||||
// — a misleading "target node is offline" error must not hide the real
|
||||
// reason the task cannot execute. Manual runs skip this; the route's
|
||||
// acting-user check is the gate. Legacy tasks (creator_user_id NULL)
|
||||
// execute as before.
|
||||
if (triggeredBy === 'scheduler' && task.creator_user_id != null) {
|
||||
const creator = db.getUserById(task.creator_user_id);
|
||||
if (!creator) {
|
||||
throw new TaskAuthorizationError('Scheduled task no longer authorized: creator account no longer exists.');
|
||||
}
|
||||
const scope = resolveTaskPermissionScope(
|
||||
task.action as BackendScheduledAction,
|
||||
task.target_type as TargetType,
|
||||
task.target_id,
|
||||
task.node_id,
|
||||
task.selector_type,
|
||||
);
|
||||
const tier: LicenseTier = LicenseService.getInstance().getTier();
|
||||
if (!checkPermissionForSubject(
|
||||
{ username: creator.username, role: creator.role, userId: creator.id },
|
||||
tier,
|
||||
scope.action,
|
||||
scope.resourceType,
|
||||
scope.resourceId,
|
||||
scope.resourceNodeId,
|
||||
)) {
|
||||
throw new TaskAuthorizationError('Scheduled task no longer authorized: creator permission was revoked.');
|
||||
}
|
||||
}
|
||||
|
||||
// Pre-check: ensure target node exists and is reachable
|
||||
if (task.node_id != null && task.action !== 'snapshot') {
|
||||
const node = db.getNode(task.node_id);
|
||||
@@ -426,6 +469,10 @@ export class SchedulerService {
|
||||
updates.enabled = 0;
|
||||
console.warn(`[SchedulerService] Task "${task.name}" (id=${task.id}) auto-disabled: cron expression invalid`);
|
||||
}
|
||||
if (error instanceof TaskAuthorizationError) {
|
||||
updates.enabled = 0;
|
||||
console.warn(`[SchedulerService] Task "${task.name}" (id=${task.id}) auto-disabled: creator authorization revoked`);
|
||||
}
|
||||
db.updateScheduledTask(task.id, updates);
|
||||
db.updateScheduledTaskRun(runId, {
|
||||
completed_at: Date.now(),
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
/**
|
||||
* Single source of truth for scheduled-operation action metadata that the
|
||||
* backend needs for validation. The route layer derives its allow-list and
|
||||
* action/target compatibility checks from this table, so adding a new action
|
||||
* means adding one entry here (plus its execution logic in SchedulerService).
|
||||
* backend needs for validation and authorization. The route layer derives its
|
||||
* allow-list, action/target compatibility checks, and permission enforcement
|
||||
* from this table, so adding a new action means adding one entry here
|
||||
* (plus its execution logic in SchedulerService).
|
||||
*
|
||||
* The frontend keeps its own richer registry (labels, categories, tones) in
|
||||
* `frontend/src/lib/scheduledActions.ts`; the two cannot share a module because
|
||||
@@ -10,6 +11,9 @@
|
||||
* each side.
|
||||
*/
|
||||
|
||||
import type { PermissionAction } from '../middleware/permissions';
|
||||
import type { ResourceType } from './DatabaseService';
|
||||
|
||||
export const VALID_TARGET_TYPES = ['stack', 'fleet', 'system', 'container'] as const;
|
||||
export type TargetType = typeof VALID_TARGET_TYPES[number];
|
||||
|
||||
@@ -19,6 +23,19 @@ export interface BackendScheduledActionDefinition {
|
||||
readonly targetTypes: readonly TargetType[];
|
||||
readonly requiresNode: boolean;
|
||||
readonly nodeScope?: 'local';
|
||||
/** Permission required to create, edit, enable, run, or delete a schedule for this action. */
|
||||
readonly permission: PermissionAction;
|
||||
}
|
||||
|
||||
/**
|
||||
* Permission scope resolved from a task's action, target, and node identity.
|
||||
* When `resourceType` is omitted the check is unscoped (global role matrix only).
|
||||
*/
|
||||
export interface ScheduledActionPermissionScope {
|
||||
readonly action: PermissionAction;
|
||||
readonly resourceType?: ResourceType;
|
||||
readonly resourceId?: string;
|
||||
readonly resourceNodeId?: number | null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -26,15 +43,15 @@ export interface BackendScheduledActionDefinition {
|
||||
* in `routes/scheduledTasks.ts` ("Must be restart, snapshot, prune, ...").
|
||||
*/
|
||||
export const BACKEND_SCHEDULED_ACTIONS = [
|
||||
{ id: 'restart', targetTypes: ['stack', 'container'], requiresNode: true },
|
||||
{ id: 'snapshot', targetTypes: ['fleet'], requiresNode: false },
|
||||
{ id: 'prune', targetTypes: ['system'], requiresNode: true, nodeScope: 'local' },
|
||||
{ id: 'update', targetTypes: ['stack', 'fleet'], requiresNode: true },
|
||||
{ id: 'scan', targetTypes: ['system'], requiresNode: true, nodeScope: 'local' },
|
||||
{ id: 'auto_backup', targetTypes: ['stack'], requiresNode: true },
|
||||
{ id: 'auto_stop', targetTypes: ['stack', 'container'], requiresNode: true },
|
||||
{ id: 'auto_down', targetTypes: ['stack'], requiresNode: true },
|
||||
{ id: 'auto_start', targetTypes: ['stack', 'container'], requiresNode: true },
|
||||
{ id: 'restart', targetTypes: ['stack', 'container'], requiresNode: true, permission: 'stack:deploy' as const },
|
||||
{ id: 'snapshot', targetTypes: ['fleet'], requiresNode: false, permission: 'node:manage' as const },
|
||||
{ id: 'prune', targetTypes: ['system'], requiresNode: true, nodeScope: 'local' as const, permission: 'system:settings' as const },
|
||||
{ id: 'update', targetTypes: ['stack', 'fleet'], requiresNode: true, permission: 'stack:deploy' as const },
|
||||
{ id: 'scan', targetTypes: ['system'], requiresNode: true, nodeScope: 'local' as const, permission: 'node:manage' as const },
|
||||
{ id: 'auto_backup',targetTypes: ['stack'], requiresNode: true, permission: 'stack:deploy' as const },
|
||||
{ id: 'auto_stop', targetTypes: ['stack', 'container'], requiresNode: true, permission: 'stack:deploy' as const },
|
||||
{ id: 'auto_down', targetTypes: ['stack'], requiresNode: true, permission: 'stack:deploy' as const },
|
||||
{ id: 'auto_start', targetTypes: ['stack', 'container'], requiresNode: true, permission: 'stack:deploy' as const },
|
||||
] as const satisfies readonly BackendScheduledActionDefinition[];
|
||||
|
||||
export type BackendScheduledAction = typeof BACKEND_SCHEDULED_ACTIONS[number]['id'];
|
||||
@@ -83,3 +100,58 @@ export function validateActionTarget(action: BackendScheduledAction, targetType:
|
||||
export function getScheduledActionDefinition(action: BackendScheduledAction): BackendScheduledActionDefinition | undefined {
|
||||
return ACTION_BY_ID.get(action);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the permission scope for a scheduled action + target combination.
|
||||
* This is the single source of truth consumed by the route layer and the
|
||||
* scheduler revalidation path. Scope resolution is per-action, not per
|
||||
* target-type bucket.
|
||||
*/
|
||||
export function resolveTaskPermissionScope(
|
||||
action: BackendScheduledAction,
|
||||
targetType: TargetType,
|
||||
targetId: string | null,
|
||||
nodeId: number | null,
|
||||
_selectorType?: string | null,
|
||||
): ScheduledActionPermissionScope {
|
||||
const def = ACTION_BY_ID.get(action);
|
||||
const basePermission = def?.permission ?? 'stack:deploy';
|
||||
|
||||
switch (action) {
|
||||
case 'restart':
|
||||
case 'auto_stop':
|
||||
case 'auto_start': {
|
||||
if (targetType === 'container') {
|
||||
return { action: 'node:manage', resourceType: 'node', resourceId: nodeId != null ? String(nodeId) : undefined, resourceNodeId: nodeId };
|
||||
}
|
||||
return { action: basePermission, resourceType: 'stack', resourceId: targetId ?? undefined, resourceNodeId: nodeId };
|
||||
}
|
||||
case 'auto_down':
|
||||
case 'auto_backup':
|
||||
return { action: basePermission, resourceType: 'stack', resourceId: targetId ?? undefined, resourceNodeId: nodeId };
|
||||
|
||||
case 'update': {
|
||||
if (targetType === 'stack') {
|
||||
return { action: basePermission, resourceType: 'stack', resourceId: targetId ?? undefined, resourceNodeId: nodeId };
|
||||
}
|
||||
if (nodeId != null) {
|
||||
return { action: 'node:manage', resourceType: 'node', resourceId: String(nodeId), resourceNodeId: nodeId };
|
||||
}
|
||||
return { action: 'node:manage' };
|
||||
}
|
||||
|
||||
case 'scan':
|
||||
return { action: basePermission, resourceType: 'node', resourceId: nodeId != null ? String(nodeId) : undefined, resourceNodeId: nodeId };
|
||||
|
||||
case 'prune':
|
||||
return { action: basePermission };
|
||||
|
||||
case 'snapshot':
|
||||
return { action: basePermission };
|
||||
|
||||
default: {
|
||||
const exhaustive: never = action;
|
||||
return { action: exhaustive as never };
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user