Skip to content

fix: add socket timeouts to postSlackReply and downloadSlackFile - #573

Merged
chaitanyagiri merged 1 commit into
HarnessMD:mainfrom
TTAWDTT:fix/bug-12-integrations-security
Oct 2, 2026
Merged

chaitanyagiri merged 1 commit into
HarnessMD:mainfrom
TTAWDTT:fix/bug-12-integrations-security

Conversation

@TTAWDTT

@TTAWDTT TTAWDTT commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

What & why

postSlackReply and downloadSlackFile used the default fetch with no timeout. A hung Slack API or CDN connection would block the calling thread indefinitely. The fix adds AbortSignal.timeout(15_000) to both calls.

Type of change

  • Bug fix

Evidence

Before

before — tests fail on main

After

after — all tests pass on fix branch

How I tested it

  • OS: Windows 11 Pro (10.0.26200), Node v24.19.0
  • Steps:
    1. Ran node --test test/slack-timeout.test.cjs against unfixed main — regression tests fail (red).
    2. Same suite on this branch — all pass (green).
    3. npm run typecheck clean.

Credit (optional)

Discord: ttawdtt

X:

Checklist

  • Before and after evidence is attached above, under both headings.
  • npm run typecheck passes.
  • npm run test:focused passes (except pre-existing Windows-env failures unrelated to this change, identical on clean main).
  • npm run build succeeds.
  • This PR is one change. Unrelated fixes belong in their own PR.
  • I read the diff myself before opening this, and there is no debug output, commented-out code, or unrelated formatting churn in it.
  • Any new UI derives from DESIGN.md / tokens.ts — no ad-hoc colors, spacing, or fonts.
  • If I added art, it's my own or compatibly licensed, and listed in ATTRIBUTION.md.

postSlackReply (src/main/slack.ts) and downloadSlackFile (src/main/index.ts)
issued raw node:https requests with no timeout anywhere. A peer that accepts
the connection but never responds leaves the promise pending forever, which
wedged the done-summary poller (its finally never ran, so slackDonePolling
stayed true for the process lifetime), hung the loopback /reply handler, and
silently dropped inbound file messages that had already been 200-acked — the
poller's designed transient-retry path never ran because a stall produces no
error at all.

Arm req.setTimeout (12s, matching fetchText.ts) that destroys the request so
the existing 'error' handlers settle the promises through the usual
transient-failure path: postSlackReply resolves ok:false (poller retries),
downloadSlackFile resolves null (attachment dropped, message still forwarded).

Regression coverage: test/slack-timeout.test.cjs drives the real
postSlackReply against a stalled local TLS endpoint (fails on unfixed code)
and pins the download timeout by source check; the full end-to-end stall
scenario (poller wedge, /reply hang, acked-then-dropped message) lives in
test/repro/bug-12.repro.cjs.
@chaitanyagiri
chaitanyagiri merged commit f44b986 into HarnessMD:main Oct 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants