Skip to content

feat: add secure webhook delivery transport#93

Merged
jmcte merged 1 commit into
mainfrom
codex/issue-55-secure-webhook
Jul 17, 2026
Merged

feat: add secure webhook delivery transport#93
jmcte merged 1 commit into
mainfrom
codex/issue-55-secure-webhook

Conversation

@jmcte

@jmcte jmcte commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restore configured webhook delivery on current main after the prior secure transport merged only into an intermediate branch and was reverted before test: restore conformance warning fixture #89 landed.
  • Require the fixed executor-owned BOOTSTRAP_NOTIFICATION_WEBHOOK_ALLOWED_HOSTS exact-host allowlist before delivery.
  • Reject URL credentials, non-443 destinations, and literal or DNS-resolved loopback, link-local, private, multicast, documentation, and reserved addresses.
  • Pin each validated public address into a dedicated HTTPS connection with shared-agent reuse disabled, and bound DNS resolution plus sequential failover under one deadline.
  • Update notification CLI help and operator documentation to match the restored secure runtime behavior.

Governing Issue

Closes #55

Validation

  • Relevant local checks passed
  • Agent-authored changes passed autoreview against the intended PR diff with no accepted/actionable findings
  • Autoreview command and result: /Users/johnteneyckjr./.codex/skills/autoreview/scripts/autoreview --mode branch --base origin/main — clean, no accepted/actionable findings; patch correct (0.90)
  • Required PR checks are expected to satisfy CI Gate
  • Skipped checks are explained below

Local checks on commit 129188f:

  • npm test -- --run tests/notifications.test.ts tests/cli.test.ts — 20 tests passed
  • npm run check — 21 test files, 119 tests passed
  • npm run build — passed
  • git diff --check origin/main...HEAD — passed
  • PR governance accounting — 262 counted production lines, below the 800-line threshold
  • No repository checks were skipped.

The first autoreview invocation also ran the test command in its isolated temporary home. Source review was clean, but tsx could not create its IPC socket because the isolated path exceeded the platform socket limit. The identical test command passed in the real checkout, and the final source-only autoreview rerun exited cleanly.

Bootstrap Governance

  • Changes are scoped to the linked issue
  • Contributor or PR guidance changes are reflected in CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md, and docs/bootstrap/onboarding.md when applicable
  • PR author enabled auto-merge where GitHub allows it, or GitHub plan-limit evidence/unavailable reason is recorded and the fallback merge-readiness policy applies
  • No real secrets, runtime auth, or machine-local env files are committed

Material change: no
ADR: not required

Merge Automation

  • PR author enabled auto-merge with gh pr merge --auto --squash, or the reason it is unavailable/unsafe is noted below

Squash auto-merge will be armed after PR creation; protected main still requires independent approval.

Notes

  • PR feat: add secure webhook delivery transport #92 contained the reviewed secure transport but merged into an intermediate integration branch and was reverted before PR test: restore conformance warning fixture #89 reached main; this replacement applies the focused transport directly to current main.
  • The built-in transport disables shared HTTPS-agent reuse with agent: false, preventing a pooled socket from bypassing the validated pinned address.
  • Failure reports remain redacted and fail closed when either required notification destination fails.

Require an executor-owned exact-host allowlist, reject non-public literal and DNS-resolved destinations, and pin validated addresses during HTTPS delivery. Bound resolution and address failover under one deadline and fail closed on transport errors.

Signed-off-by: Your Name <you@example.com>
@jmcte
jmcte requested a review from athena-omt July 17, 2026 23:19
@jmcte
jmcte enabled auto-merge (squash) July 17, 2026 23:19
@athena-omt athena-omt added status:needs-review PR is ready for Athena review. review:athena Athena review governance requested. state:waiting-checks Waiting for CI/check status to settle. and removed state:waiting-checks Waiting for CI/check status to settle. labels Jul 17, 2026

@athena-omt athena-omt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved for the current head. The transport enforces the executor-owned exact-host allowlist, HTTPS/443, credential rejection, public-address DNS validation, and pinned-address HTTPS connections with a bounded deadline. Reviewed the CLI call path and notification tests; no unresolved threads or prior reviews. Validation passed: targeted notifications/CLI tests (20), full check suite (119), build, and both published workflows (PR Fast CI and AI Attestation).

@jmcte
jmcte merged commit 3d2f360 into main Jul 17, 2026
9 checks passed
@jmcte
jmcte deleted the codex/issue-55-secure-webhook branch July 17, 2026 23:30
@athena-omt athena-omt removed status:needs-review PR is ready for Athena review. review:athena Athena review governance requested. labels Jul 17, 2026
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.

Validate exceptions and emit material-action notifications

2 participants