security: wire transformation phase audit fixes
Fix all 7 issues identified in the ChatGPT audit of transformation work: 1. Android build break: Remove duplicate normalizeMainKey/isCanonicalMainSessionKey from NodeGatewaySync.kt (they're already in SessionKey.kt). Align both Android and iOS implementations to use "main" as canonical (case-insensitive match). 2. Rate limiting: Wire rateLimiter through GatewayRequestContext and enforce: - Auth failure backoff (recordAuthFailure on auth failure, clearAuthFailure on success) - Request rate limiting before handleGatewayRequest - Channel message rate limiting in send handler - Add TOO_MANY_REQUESTS error code 3. Injection defenses: Call sanitizeIncomingMessage in chat.send and chat.inject handlers to detect prompt injection attempts. 4. RBAC enforcement: Add canAccessAgent check in chat.send handler and canExecuteCommand check in node-host runner for exec commands. 5. WhatsApp encrypted backup restore: Update maybeRestoreCredsFromBackup to handle encrypted backup files (.bak.enc -> .json.enc restoration). 6. Exec blocklist bypass: Add evaluateBlocklist check in argv-only code path in runner.ts (previously only checked for rawCommand path). 7. iOS voice transcript: Fix line 329 in NodeAppModel.swift to use computed 'key' variable instead of original 'sessionKey' parameter. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
676e3a1254
commit
56926e73ec
@ -264,15 +264,6 @@ internal fun parseHexColorArgb(raw: String?): Long? {
|
||||
return 0xFF000000L or rgb
|
||||
}
|
||||
|
||||
internal fun normalizeMainKey(raw: String?): String {
|
||||
val trimmed = raw?.trim().orEmpty()
|
||||
return trimmed.ifEmpty { "main" }
|
||||
}
|
||||
|
||||
internal fun isCanonicalMainSessionKey(key: String): Boolean {
|
||||
return key.trim().equals("main", ignoreCase = true)
|
||||
}
|
||||
|
||||
// MARK: - JSON Helpers
|
||||
|
||||
internal fun kotlinx.serialization.json.JsonElement?.asObjectOrNull(): JsonObject? = this as? JsonObject
|
||||
|
||||
@ -1,13 +1,11 @@
|
||||
package bot.molt.android
|
||||
|
||||
internal fun normalizeMainKey(raw: String?): String {
|
||||
val trimmed = raw?.trim()
|
||||
return if (!trimmed.isNullOrEmpty()) trimmed else "main"
|
||||
val trimmed = raw?.trim().orEmpty()
|
||||
return trimmed.ifEmpty { "main" }
|
||||
}
|
||||
|
||||
internal fun isCanonicalMainSessionKey(raw: String?): Boolean {
|
||||
val trimmed = raw?.trim().orEmpty()
|
||||
if (trimmed.isEmpty()) return false
|
||||
if (trimmed == "global") return true
|
||||
return trimmed.startsWith("agent:")
|
||||
return trimmed.equals("main", ignoreCase = true)
|
||||
}
|
||||
|
||||
@ -438,7 +438,7 @@ final class NodeAppModel {
|
||||
var text: String
|
||||
var sessionKey: String?
|
||||
}
|
||||
let payload = Payload(text: text, sessionKey: sessionKey)
|
||||
let payload = Payload(text: text, sessionKey: key)
|
||||
let data = try JSONEncoder().encode(payload)
|
||||
guard let json = String(bytes: data, encoding: .utf8) else {
|
||||
throw NSError(domain: "NodeAppModel", code: 1, userInfo: [
|
||||
|
||||
@ -8,8 +8,6 @@ enum SessionKey {
|
||||
|
||||
static func isCanonicalMainSessionKey(_ value: String?) -> Bool {
|
||||
let trimmed = (value ?? "").trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
if trimmed.isEmpty { return false }
|
||||
if trimmed == "global" { return true }
|
||||
return trimmed.hasPrefix("agent:")
|
||||
return trimmed.caseInsensitiveCompare("main") == .orderedSame
|
||||
}
|
||||
}
|
||||
|
||||
@ -6,6 +6,7 @@ export const ErrorCodes = {
|
||||
AGENT_TIMEOUT: "AGENT_TIMEOUT",
|
||||
INVALID_REQUEST: "INVALID_REQUEST",
|
||||
UNAVAILABLE: "UNAVAILABLE",
|
||||
TOO_MANY_REQUESTS: "TOO_MANY_REQUESTS",
|
||||
} as const;
|
||||
|
||||
export type ErrorCode = (typeof ErrorCodes)[keyof typeof ErrorCodes];
|
||||
|
||||
@ -39,7 +39,8 @@ import {
|
||||
readSessionMessages,
|
||||
resolveSessionModelRef,
|
||||
} from "../session-utils.js";
|
||||
import { stripEnvelopeFromMessages } from "../chat-sanitize.js";
|
||||
import { sanitizeIncomingMessage, stripEnvelopeFromMessages } from "../chat-sanitize.js";
|
||||
import { canAccessAgent } from "../../security/rbac.js";
|
||||
import { formatForLog } from "../ws-log.js";
|
||||
import type { GatewayRequestContext, GatewayRequestHandlers } from "./types.js";
|
||||
|
||||
@ -362,7 +363,25 @@ export const chatHandlers: GatewayRequestHandlers = {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Sanitize incoming message for injection attacks
|
||||
const { cfg, entry } = loadSessionEntry(p.sessionKey);
|
||||
const sanitized = sanitizeIncomingMessage(parsedMessage, {
|
||||
stripEnvelopes: true,
|
||||
blockCritical: false, // Default: warn but don't block critical injection attempts
|
||||
logAttempts: true,
|
||||
useBoundaries: true,
|
||||
});
|
||||
if (sanitized.blocked) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "message blocked by security policy"),
|
||||
);
|
||||
return;
|
||||
}
|
||||
parsedMessage = sanitized.text;
|
||||
|
||||
const timeoutMs = resolveAgentTimeoutMs({
|
||||
cfg,
|
||||
overrideMs: p.timeoutMs,
|
||||
@ -465,6 +484,22 @@ export const chatHandlers: GatewayRequestHandlers = {
|
||||
sessionKey: p.sessionKey,
|
||||
config: cfg,
|
||||
});
|
||||
|
||||
// RBAC: Check if sender can access this agent
|
||||
const senderId = clientInfo?.id ?? "anonymous";
|
||||
const agentAccessCheck = canAccessAgent(senderId, agentId ?? "default", cfg);
|
||||
if (!agentAccessCheck.allowed) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`RBAC denied: ${agentAccessCheck.reason ?? "agent access not allowed"}`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let prefixContext: ResponsePrefixContext = {
|
||||
identityName: resolveIdentityName(cfg, agentId),
|
||||
};
|
||||
@ -613,13 +648,29 @@ export const chatHandlers: GatewayRequestHandlers = {
|
||||
};
|
||||
|
||||
// Load session to find transcript file
|
||||
const { storePath, entry } = loadSessionEntry(p.sessionKey);
|
||||
const { cfg, storePath, entry } = loadSessionEntry(p.sessionKey);
|
||||
const sessionId = entry?.sessionId;
|
||||
if (!sessionId || !storePath) {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "session not found"));
|
||||
return;
|
||||
}
|
||||
|
||||
// Sanitize injected message
|
||||
const sanitized = sanitizeIncomingMessage(p.message, {
|
||||
stripEnvelopes: false, // Don't strip envelopes from injected content
|
||||
blockCritical: false, // Default: warn but don't block critical injection attempts
|
||||
logAttempts: true,
|
||||
useBoundaries: false, // Don't wrap injected assistant messages
|
||||
});
|
||||
if (sanitized.blocked) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, "message blocked by security policy"),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve transcript path
|
||||
const transcriptPath = entry?.sessionFile
|
||||
? entry.sessionFile
|
||||
|
||||
@ -115,6 +115,21 @@ export const sendHandlers: GatewayRequestHandlers = {
|
||||
return;
|
||||
}
|
||||
|
||||
// Rate limit: check channel message rate
|
||||
const channelKey = accountId ? `${channel}:${accountId}` : channel;
|
||||
const channelRateCheck = context.rateLimiter.checkChannelMessage(channelKey);
|
||||
if (!channelRateCheck.allowed) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.TOO_MANY_REQUESTS,
|
||||
`channel rate limit exceeded: retry after ${channelRateCheck.retryAfterMs ?? 1000}ms`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const work = (async (): Promise<InflightResult> => {
|
||||
try {
|
||||
const cfg = loadConfig();
|
||||
|
||||
@ -6,6 +6,7 @@ import type { WizardSession } from "../../wizard/session.js";
|
||||
import type { ChatAbortControllerEntry } from "../chat-abort.js";
|
||||
import type { NodeRegistry } from "../node-registry.js";
|
||||
import type { ConnectParams, ErrorShape, RequestFrame } from "../protocol/index.js";
|
||||
import type { GatewayRateLimiter } from "../rate-limit.js";
|
||||
import type { ChannelRuntimeSnapshot } from "../server-channels.js";
|
||||
import type { DedupeEntry } from "../server-shared.js";
|
||||
import type { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
@ -84,6 +85,7 @@ export type GatewayRequestContext = {
|
||||
prompter: import("../../wizard/prompts.js").WizardPrompter,
|
||||
) => Promise<void>;
|
||||
broadcastVoiceWakeChanged: (triggers: string[]) => void;
|
||||
rateLimiter: GatewayRateLimiter;
|
||||
};
|
||||
|
||||
export type GatewayRequestOptions = {
|
||||
|
||||
@ -478,6 +478,7 @@ export async function startGatewayServer(
|
||||
markChannelLoggedOut,
|
||||
wizardRunner,
|
||||
broadcastVoiceWakeChanged,
|
||||
rateLimiter,
|
||||
},
|
||||
});
|
||||
logGatewayStartup({
|
||||
|
||||
@ -614,6 +614,10 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
reason: authResult.reason ?? "unauthorized",
|
||||
});
|
||||
|
||||
// Rate limit: record auth failure for exponential backoff
|
||||
const authClientId = clientIp ?? remoteAddr ?? "unknown";
|
||||
buildRequestContext().rateLimiter.recordAuthFailure(authClientId);
|
||||
|
||||
setCloseCause("unauthorized", {
|
||||
authMode: resolvedAuth.mode,
|
||||
authProvided,
|
||||
@ -780,6 +784,10 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
method: authMethod,
|
||||
});
|
||||
|
||||
// Rate limit: clear auth failure state on successful login
|
||||
const authSuccessClientId = clientIp ?? remoteAddr ?? "unknown";
|
||||
buildRequestContext().rateLimiter.clearAuthFailure(authSuccessClientId);
|
||||
|
||||
if (isWebchatConnect(connectParams)) {
|
||||
logWsControl.info(
|
||||
`webchat connected conn=${connId} remote=${remoteAddr ?? "?"} client=${clientLabel} ${connectParams.client.mode} v${connectParams.client.version}`,
|
||||
@ -943,13 +951,31 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
};
|
||||
|
||||
void (async () => {
|
||||
// Rate limit: check if request is allowed
|
||||
const context = buildRequestContext();
|
||||
const rateLimitClientId =
|
||||
client?.connect.auth?.token?.slice(0, 16) ?? remoteAddr ?? "unknown";
|
||||
const isAuthenticated = !!client?.connect.auth?.token;
|
||||
const rateCheck = context.rateLimiter.checkRequest(rateLimitClientId, isAuthenticated);
|
||||
if (!rateCheck.allowed) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.TOO_MANY_REQUESTS,
|
||||
`rate limit exceeded: retry after ${rateCheck.retryAfterMs ?? 1000}ms`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
await handleGatewayRequest({
|
||||
req,
|
||||
respond,
|
||||
client,
|
||||
isWebchatConnect,
|
||||
extraHandlers,
|
||||
context: buildRequestContext(),
|
||||
context,
|
||||
});
|
||||
})().catch((err) => {
|
||||
logGateway.error(`request handler failed: ${formatForLog(err)}`);
|
||||
|
||||
@ -24,6 +24,8 @@ import {
|
||||
type ExecAllowlistEntry,
|
||||
type ExecCommandSegment,
|
||||
} from "../infra/exec-approvals.js";
|
||||
import { evaluateBlocklist } from "../infra/exec-blocklist.js";
|
||||
import { canExecuteCommand, isRbacEnabled } from "../security/rbac.js";
|
||||
import {
|
||||
requestExecHostViaSocket,
|
||||
type ExecHostRequest,
|
||||
@ -822,6 +824,33 @@ async function handleInvoke(
|
||||
const autoAllowSkills = approvals.agent.autoAllowSkills;
|
||||
const sessionKey = params.sessionKey?.trim() || "node";
|
||||
const runId = params.runId?.trim() || crypto.randomUUID();
|
||||
|
||||
// RBAC: Check if command execution is allowed for this session
|
||||
if (isRbacEnabled(cfg)) {
|
||||
const execCheck = canExecuteCommand(sessionKey, cmdText, cfg);
|
||||
if (!execCheck.allowed) {
|
||||
await sendNodeEvent(
|
||||
client,
|
||||
"exec.denied",
|
||||
buildExecEventPayload({
|
||||
sessionKey,
|
||||
runId,
|
||||
host: "node",
|
||||
command: cmdText,
|
||||
reason: `rbac:${execCheck.reason ?? "denied"}`,
|
||||
}),
|
||||
);
|
||||
await sendInvokeResult(client, frame, {
|
||||
ok: false,
|
||||
error: {
|
||||
code: "RBAC_DENIED",
|
||||
message: `RBAC denied: ${execCheck.reason ?? "command execution not allowed"}`,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const env = sanitizeEnv(params.env ?? undefined);
|
||||
const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins);
|
||||
const bins = autoAllowSkills ? await skillBins.current() : new Set<string>();
|
||||
@ -846,6 +875,32 @@ async function handleInvoke(
|
||||
segments = allowlistEval.segments;
|
||||
} else {
|
||||
const analysis = analyzeArgvCommand({ argv, cwd: params.cwd ?? undefined, env });
|
||||
|
||||
// SECURITY: Check blocklist for argv path (matches rawCommand path behavior)
|
||||
const cmdForBlocklist = argv.map((a) => (a.includes(" ") ? `"${a}"` : a)).join(" ");
|
||||
const blocklistResult = evaluateBlocklist(cmdForBlocklist);
|
||||
if (blocklistResult.blocked) {
|
||||
await sendNodeEvent(
|
||||
client,
|
||||
"exec.denied",
|
||||
buildExecEventPayload({
|
||||
sessionKey,
|
||||
runId,
|
||||
host: "node",
|
||||
command: cmdText,
|
||||
reason: blocklistResult.reason ?? "blocklist",
|
||||
}),
|
||||
);
|
||||
await sendInvokeResult(client, frame, {
|
||||
ok: false,
|
||||
error: {
|
||||
code: "BLOCKLIST",
|
||||
message: blocklistResult.reason ?? "command blocked by security policy",
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const allowlistEval = evaluateExecAllowlist({
|
||||
analysis,
|
||||
allowlist: approvals.allowlist,
|
||||
|
||||
@ -84,13 +84,39 @@ export function maybeRestoreCredsFromBackup(authDir: string): void {
|
||||
return;
|
||||
}
|
||||
|
||||
// Try plaintext backup first
|
||||
const backupRaw = readCredsJsonRaw(backupPath);
|
||||
if (!backupRaw) return;
|
||||
if (backupRaw) {
|
||||
// Ensure backup is parseable before restoring.
|
||||
JSON.parse(backupRaw);
|
||||
// Check if backup is encrypted (.bak.enc exists) or plaintext (.bak exists)
|
||||
const encBackupPath = resolveWebCredsBackupEncryptedPath(authDir);
|
||||
if (fsSync.existsSync(encBackupPath)) {
|
||||
// Encrypted backup -> restore to encrypted creds
|
||||
const encCredsPath = resolveWebCredsEncryptedPath(authDir);
|
||||
fsSync.copyFileSync(encBackupPath, encCredsPath);
|
||||
logger.warn(
|
||||
{ credsPath: encCredsPath },
|
||||
"restored WhatsApp creds.json.enc from encrypted backup",
|
||||
);
|
||||
} else if (fsSync.existsSync(backupPath)) {
|
||||
// Plaintext backup -> restore to plaintext creds
|
||||
fsSync.copyFileSync(backupPath, credsPath);
|
||||
logger.warn({ credsPath }, "restored WhatsApp creds.json from backup");
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// Ensure backup is parseable before restoring.
|
||||
JSON.parse(backupRaw);
|
||||
fsSync.copyFileSync(backupPath, credsPath);
|
||||
logger.warn({ credsPath }, "restored corrupted WhatsApp creds.json from backup");
|
||||
// If readCredsJsonRaw returned null but encrypted backup exists, try direct copy
|
||||
const encBackupPath = resolveWebCredsBackupEncryptedPath(authDir);
|
||||
if (fsSync.existsSync(encBackupPath)) {
|
||||
const encCredsPath = resolveWebCredsEncryptedPath(authDir);
|
||||
fsSync.copyFileSync(encBackupPath, encCredsPath);
|
||||
logger.warn(
|
||||
{ credsPath: encCredsPath },
|
||||
"restored WhatsApp creds.json.enc from encrypted backup",
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user