Skip to content
Merged
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
17 changes: 15 additions & 2 deletions slack-bridge/broker-bridge.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
validateSendParams,
validateReactParams,
createRateLimiter,
sanitizeOutboundText,
} from "./security.mjs";
import {
canonicalizeEnvelope,
Expand Down Expand Up @@ -575,6 +576,16 @@ async function _react(channel, threadTs, emoji) {
});
}

function sanitizeOutboundMessage(text, contextLabel) {
const sanitized = sanitizeOutboundText(text);
if (sanitized.blocked) {
logWarn(`🛡️ outbound message blocked (${contextLabel}): ${sanitized.reasons.join(", ")}`);
} else if (sanitized.redacted) {
logWarn(`🧼 outbound message redacted (${contextLabel}): ${sanitized.reasons.join(", ")}`);
}
return sanitized.text;
}

async function handleUserMessage(userMessage, event) {
logInfo(`👤 message from <@${event.user}> in ${event.channel} (type: ${event.type}, ts: ${event.ts})`);

Expand Down Expand Up @@ -849,11 +860,12 @@ function startApiServer() {
}

const { channel, text, thread_ts } = apiRequestBody;
const safeText = sanitizeOutboundMessage(text, "/send");

const result = await sendViaBroker({
action: "chat.postMessage",
routing: { channel, ...(thread_ts ? { thread_ts } : {}) },
actionRequestBody: { text },
actionRequestBody: { text: safeText },
});

res.writeHead(200, { "Content-Type": "application/json" });
Expand Down Expand Up @@ -881,10 +893,11 @@ function startApiServer() {
return;
}

const safeText = sanitizeOutboundMessage(text, "/reply");
const result = await sendViaBroker({
action: "chat.postMessage",
routing: { channel: thread.channel, thread_ts: thread.thread_ts },
actionRequestBody: { text },
actionRequestBody: { text: safeText },
});

res.writeHead(200, { "Content-Type": "application/json" });
Expand Down
51 changes: 51 additions & 0 deletions slack-bridge/security.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,57 @@ export function wrapExternalContent({ text, source, user, channel, threadTs }) {
].join("\n");
}

// ── Outbound Redaction / Leak Prevention ───────────────────────────────────

const OUTBOUND_BLOCK_PATTERNS = [
{ pattern: /\/proc\/(self|\d+)\/environ\b/i, reason: "proc-environ-path" },
{ pattern: /\/proc\/(self|\d+)\/cmdline\b/i, reason: "proc-cmdline-path" },
{ pattern: /\0[A-Za-z_][A-Za-z0-9_]*=[^\0]{0,200}\0/, reason: "nul-delimited-env-dump" },
];

const OUTBOUND_REDACT_PATTERNS = [
{ pattern: /\bxox[baprs]-[0-9A-Za-z-]{12,}\b/g, replacement: "[REDACTED_SLACK_TOKEN]", reason: "slack-token" },
{ pattern: /\bgh[pousr]_[A-Za-z0-9_]{20,}\b/g, replacement: "[REDACTED_GITHUB_TOKEN]", reason: "github-token" },
{ pattern: /\bgithub_pat_[A-Za-z0-9_]{20,}\b/g, replacement: "[REDACTED_GITHUB_TOKEN]", reason: "github-token" },
{ pattern: /\bsk-[A-Za-z0-9]{20,}\b/g, replacement: "[REDACTED_API_KEY]", reason: "openai-key" },
{ pattern: /\bAKIA[A-Z0-9]{16}\b/g, replacement: "[REDACTED_AWS_KEY]", reason: "aws-access-key" },
{ pattern: /\b((?:SECRET|TOKEN|PASSWORD|PASS|API(?:_|-)?KEY|ACCESS(?:_|-)?KEY|PRIVATE(?:_|-)?KEY|SESSION|COOKIE|BEARER|SLACK|GITHUB|OPENAI|ANTHROPIC|GEMINI|AWS)[A-Z0-9_-]*)=[^\s\n\r\0]{1,400}/gi, replacement: "$1=[REDACTED_ENV]", reason: "sensitive-env-assignment" },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The character class [^\s\n\r\0] is redundant since \s already includes \n and \r. This could be simplified to [^\s\0] for cleaner regex syntax.

Prompt To Fix With AI
This is a comment left during a code review.
Path: slack-bridge/security.mjs
Line: 178

Comment:
The character class `[^\s\n\r\0]` is redundant since `\s` already includes `\n` and `\r`. This could be simplified to `[^\s\0]` for cleaner regex syntax.

How can I resolve this? If you propose a fix, please make it concise.

];

const OUTBOUND_BLOCK_FALLBACK = "I found potentially sensitive runtime data and omitted it. I can still help with the task if you share only the non-sensitive details.";

export function sanitizeOutboundText(input) {
let text = typeof input === "string" ? input : String(input);
const reasons = [];

for (const rule of OUTBOUND_BLOCK_PATTERNS) {
if (rule.pattern.test(text)) {
reasons.push(rule.reason);
}
}

if (reasons.length > 0) {
return {
text: OUTBOUND_BLOCK_FALLBACK,
redacted: true,
blocked: true,
reasons,
};
}

let redacted = false;
for (const rule of OUTBOUND_REDACT_PATTERNS) {
const next = text.replace(rule.pattern, rule.replacement);
if (next !== text) {
redacted = true;
reasons.push(rule.reason);
text = next;
}
}

return { text, redacted, blocked: false, reasons };
}

// ── Access Control ──────────────────────────────────────────────────────────

/**
Expand Down
38 changes: 38 additions & 0 deletions slack-bridge/security.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
validateReactParams,
safeEqualSecret,
createRateLimiter,
sanitizeOutboundText,
} from "./security.mjs";

// ── detectSuspiciousPatterns ────────────────────────────────────────────────
Expand Down Expand Up @@ -315,6 +316,43 @@ describe("formatForSlack", () => {
});
});

// ── sanitizeOutboundText ────────────────────────────────────────────────────

describe("sanitizeOutboundText", () => {
it("passes through clean text", () => {
const result = sanitizeOutboundText("All good here.");
assert.equal(result.text, "All good here.");
assert.equal(result.redacted, false);
assert.equal(result.blocked, false);
assert.deepEqual(result.reasons, []);
});

it("blocks /proc environ references", () => {
const result = sanitizeOutboundText("Saw this in /proc/self/environ just now");
assert.equal(result.blocked, true);
assert.equal(result.redacted, true);
assert.ok(result.text.includes("omitted"));
assert.ok(result.reasons.includes("proc-environ-path"));
});

it("redacts sensitive env assignments", () => {
const result = sanitizeOutboundText("OPENAI_API_KEY=sk-abcdefghijklmnopqrstuvwxyz123456");
assert.equal(result.blocked, false);
assert.equal(result.redacted, true);
assert.equal(result.text, "OPENAI_API_KEY=[REDACTED_ENV]");
assert.ok(result.reasons.includes("sensitive-env-assignment"));
});

it("redacts known token formats", () => {
const syntheticSlackToken = `xox${"b"}-123456789012-abcdefghijklmno`;
const result = sanitizeOutboundText(`token ${syntheticSlackToken}`);
assert.equal(result.blocked, false);
assert.equal(result.redacted, true);
assert.ok(result.text.includes("[REDACTED_SLACK_TOKEN]"));
assert.ok(result.reasons.includes("slack-token"));
});
});

// ── validateSendParams ──────────────────────────────────────────────────────

describe("validateSendParams", () => {
Expand Down