Skip to content

Incoming webhook authentication (HMAC / bearer / token / Ed25519 with provider presets) - #4

Open
zachbroad wants to merge 17 commits into
mainfrom
feat/webhook-auth
Open

Incoming webhook authentication (HMAC / bearer / token / Ed25519 with provider presets)#4
zachbroad wants to merge 17 commits into
mainfrom
feat/webhook-auth

Conversation

@zachbroad

@zachbroad zachbroad commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Addresses #1.

A preset-driven verifier that authenticates incoming webhooks before they are stored or fanned out, rejecting unauthenticated requests with 401. Centers HMAC signature verification (per the discussion in #1) while also supporting bearer/token and Ed25519, with per-provider presets so a typical setup is "pick a preset, paste the secret."

What's included

  • internal/inboundauth — a parameterized verifier over four independent axes (algorithm sha256/sha1, encoding hex/base64, header/prefix parser, signed-string template) plus timestamp-tolerance for replay-bound schemes. Schemes: none (default), hmac, bearer, token, ed25519.
  • 8 provider presets — GitHub, Forgejo/Gitea, Stripe, Slack, Shopify, Svix/Standard-Webhooks, GitLab (token), Discord (Ed25519). Verified against real signature vectors, including GitHub's documented example.
  • Persistencesources.auth_config JSONB column (migration 000009); secrets stored plaintext (encryption is a tracked follow-up).
  • Ingest gate — verification runs in handler/webhook.go after body-read and before Deliveries.Create; on failure nothing is persisted or published, 401 is returned, and nitrohook_webhook_auth_failures_total{source,reason} is incremented.
  • Web UI — an "Authentication" auth-card on the source Overview tab (mirrors the existing mode-card htmx pattern): enable, pick a preset, paste the secret/public key, save.
  • Docsguides/authentication.md + CHANGELOG + CLAUDE.md package entry.

Security notes

  • All comparisons are constant-time (hmac.Equal on decoded bytes, subtle.ConstantTimeCompare, ed25519.Verify); base64 is decoded rather than string-compared to avoid case-folding false-accepts.
  • Timestamp bound into the signature is the exact value checked against the tolerance window (no desync).
  • Verifiers fail closed on an empty credential, and the UI refuses to overwrite a stored secret with a blank submission (a whole-branch review caught this as a real forgeability path and it was fixed + re-verified).
  • Backward compatible: sources with no auth_config behave exactly as before.

Testing

  • inboundauth unit suite (all schemes + presets, real vectors) — green.
  • Handler integration: no-signature → 401 with zero deliveries stored; valid HMAC → 202. Store auth round-trip — green.
  • Note: the pre-existing TestDeliveryLifecycle failure (a deliveries-payload JSONB-whitespace assertion) is unrelated to this branch — it touches no auth code — and is left for a separate fix.

Deferred follow-ups

Secret rotation, HTTPS-only enforcement, encryption-at-rest, a "Custom" scheme axis editor, and a Twilio preset (its URL+params signing doesn't fit the current axes).

Design docs

  • Spec: docs/superpowers/specs/2026-07-10-incoming-webhook-authentication-design.md
  • Plan: docs/superpowers/plans/2026-07-10-incoming-webhook-authentication.md

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added per-source incoming webhook authentication with HMAC, bearer/token, and Ed25519 verification using provider-specific presets.
    • Added an authentication configuration UI for sources and a route to save/enable/disable webhook auth.
    • Added an optional MCP server exposing read-only NitroHook data.
  • Bug Fixes
    • Improved webhook payload comparisons in integration/end-to-end tests for more reliable JSON assertions.
  • Documentation
    • Documented webhook authentication, presets, security best practices, and auth failure monitoring.
    • Updated quickstart/tour and MCP server guide.
  • Chores
    • Added Makefile development workflows (make dev-setup, make dev, make dev-down) and updated build/testing commands.

zachbroad and others added 15 commits July 10, 2026 19:25
Preset-driven signature verifier over a parameterized core: HMAC (SHA256/SHA1,
hex/base64, configurable header/prefix/template), Bearer, plain-token (GitLab),
and Ed25519 (Discord). Presets for GitHub, Forgejo, Stripe, Slack, Shopify, Svix,
GitLab, Discord, plus Custom. Verifies before persist/publish; 401 + Prometheus
counter on failure. Secret stored plaintext (encryption deferred).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `make dev` for a one-command local development loop: brings up
Postgres + Redis in Docker (waiting until healthy), applies migrations
via the API binary, then runs the API with an in-process worker under
Air for hot reload. Add `make dev-setup` (installs Air) and `make
dev-down` (stops the infra).

Start CHANGELOG.md (Keep a Changelog + SemVer) with a 0.1.0 entry, and
document changelog + docs-site upkeep expectations in CLAUDE.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire inboundauth verification into the ingest handler so unauthenticated
or badly-signed webhooks are rejected with 401 before any delivery row
is created or published to Redis. Adds a nitrohook_webhook_auth_failures_total
counter (source, reason) for observability.
Re-commits Task 1's persistence layer, which was orphaned from branch
history by an intervening amend+reset; content is identical to the
reviewed commit 20ad733.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds an auth-card htmx fragment on the source overview page (mirroring
mode-card) with a preset dropdown and secret field, backed by a new
POST /sources/:slug/auth handler that saves config via
store.SetAuthConfig and re-renders the fragment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds per-source webhook authentication with configurable HMAC, token, bearer, and Ed25519 schemes; persists configuration, enforces verification before delivery storage, exposes failure metrics, adds source UI configuration, introduces a read-only MCP server, updates development tooling, and refreshes bundled Monaco assets.

Changes

Incoming webhook authentication

Layer / File(s) Summary
Authentication storage and verification
migrations/*, internal/model/model.go, internal/store/*, internal/inboundauth/*
Adds JSONB-backed source authentication configuration, preset-based HMAC/token/bearer/Ed25519 verification, failure classification, and unit/integration coverage.
Webhook verification gate
internal/handler/webhook.go, internal/handler/webhook_auth_integration_test.go, internal/metrics/metrics.go
Verifies requests before delivery creation, returns HTTP 401 on failures, increments labeled authentication metrics, and tests rejected and accepted requests.
Source authentication UI
cmd/api/main.go, web/handler.go, web/source.go, web/templates/source-overview.html
Adds source authentication state, preset validation, persistence handling, HTMX rendering, and the authentication route.
Authentication documentation
docs/src/content/docs/guides/authentication.md, docs/superpowers/*, CHANGELOG.md, CLAUDE.md
Documents supported schemes, provider presets, monitoring behavior, implementation design, repository guidance, and release notes.

MCP server and development workflow

Layer / File(s) Summary
MCP server
cmd/mcp/main.go, Makefile, README.md, docs/src/content/docs/guides/mcp.md
Adds a stdio MCP server with read-only source, action, and delivery tools, plus build and usage documentation.
Local development and tests
Makefile, docker-compose.yml, internal/testutil/testutil.go, internal/*integration_test.go, .gitignore
Adds Docker-backed development targets, Grafana persistence, serialized integration execution, structural JSON assertions, and local tooling ignores.
Documentation and dependencies
README.md, docs/*, go.mod
Updates development, quickstart, tour, navigation, MCP documentation, and module requirements.
Bundled Monaco assets
web/static/vs/*
Refreshes Monaco loader, worker, JavaScript/TypeScript language modules, TypeScript services, and editor styles.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: preset-driven incoming webhook authentication across the supported schemes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/webhook-auth

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.44.1)
web/static/vs/editor/editor.main.js

ast-grep timed out on this file

web/static/vs/language/typescript/tsWorker.js

ast-grep timed out on this file

web/static/vs/base/worker/workerMain.js

ast-grep timed out on this file


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (2)
web/source.go (1)

479-485: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a direct PresetInfo lookup to avoid the linear scan.

The preset is already fetched via inboundauth.Preset(presetName) at line 472, then separately scanned for via PresetNames() at lines 479-485 to get NeedsSecret/NeedsPublicKey flags. A dedicated inboundauth.PresetInfo(name) (or having Preset return both) would eliminate this duplication.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/source.go` around lines 479 - 485, Replace the linear PresetNames() scan
in the preset-handling logic with a direct inboundauth.PresetInfo(presetName)
lookup, or update inboundauth.Preset to return the preset metadata alongside the
preset value; use the returned NeedsSecret and NeedsPublicKey fields without
iterating through all preset names.
Makefile (1)

18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the Air version used by the development workflow.

Using @latest makes make dev-setup non-reproducible and can unexpectedly pull a version incompatible with the repository’s Go toolchain. Pin the version validated by CI/local development.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` at line 18, Replace the `@latest` suffix in the Air installation
command used by the development setup with the specific version validated by CI
and local development, keeping the pinned version consistent across the
repository.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/src/content/docs/guides/authentication.md`:
- Line 50: Update the reason label list in the authentication guide to include
missing_secret, matching the ErrMissingSecret mapping in config.go and placing
it alongside the other possible verification outcomes.

In `@internal/inboundauth/presets.go`:
- Around line 42-45: Add TSToleranceS: 300 to the "discord-ed25519" preset
alongside TSHeader, matching the timestamp freshness configuration used by the
Stripe, Slack, and Svix presets.

In `@internal/inboundauth/verify.go`:
- Around line 228-243: verifyBearer must reject Authorization values without a
valid Bearer scheme and accept the scheme case-insensitively. Replace the
unconditional strings.TrimPrefix call with explicit parsing that validates the
prefix using case-insensitive matching, extracts only the token after the
required separator, and returns ErrBadSignature for other schemes before
performing the constant-time token comparison.

In `@internal/store/source_auth_integration_test.go`:
- Around line 51-53: Check and handle the error returned by the final
s.Sources.GetBySlug call before dereferencing cleared, matching the error
handling used by the earlier GetBySlug calls; fail the test with a clear message
if retrieval fails, then validate cleared.AuthConfig.

In `@web/source.go`:
- Line 500: Handle both errors in UpdateSourceAuth instead of discarding them:
check json.Marshal(base) and return the appropriate error before calling
SetAuthConfig, then check GetBySlug and return its error before rendering the
template. Ensure failures do not persist a nil auth configuration or continue
with a nil source.

---

Nitpick comments:
In `@Makefile`:
- Line 18: Replace the `@latest` suffix in the Air installation command used by
the development setup with the specific version validated by CI and local
development, keeping the pinned version consistent across the repository.

In `@web/source.go`:
- Around line 479-485: Replace the linear PresetNames() scan in the
preset-handling logic with a direct inboundauth.PresetInfo(presetName) lookup,
or update inboundauth.Preset to return the preset metadata alongside the preset
value; use the returned NeedsSecret and NeedsPublicKey fields without iterating
through all preset names.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 48c5bd21-c243-4f3d-bf4a-847c2bf8a064

📥 Commits

Reviewing files that changed from the base of the PR and between 8ed5ac0 and dad5918.

⛔ Files ignored due to path filters (6)
  • docs/public/screenshots/actions.png is excluded by !**/*.png
  • docs/public/screenshots/deliveries.png is excluded by !**/*.png
  • docs/public/screenshots/delivery-detail.png is excluded by !**/*.png
  • docs/public/screenshots/script-editor.png is excluded by !**/*.png
  • docs/public/screenshots/source-overview.png is excluded by !**/*.png
  • docs/public/screenshots/sources.png is excluded by !**/*.png
📒 Files selected for processing (25)
  • CHANGELOG.md
  • CLAUDE.md
  • Makefile
  • README.md
  • cmd/api/main.go
  • docs/src/content/docs/guides/authentication.md
  • docs/superpowers/plans/2026-07-10-incoming-webhook-authentication.md
  • docs/superpowers/specs/2026-07-10-incoming-webhook-authentication-design.md
  • internal/handler/handler_integration_test.go
  • internal/handler/webhook.go
  • internal/handler/webhook_auth_integration_test.go
  • internal/inboundauth/config.go
  • internal/inboundauth/presets.go
  • internal/inboundauth/presets_test.go
  • internal/inboundauth/verify.go
  • internal/inboundauth/verify_test.go
  • internal/metrics/metrics.go
  • internal/model/model.go
  • internal/store/source.go
  • internal/store/source_auth_integration_test.go
  • migrations/000009_add_source_auth.down.sql
  • migrations/000009_add_source_auth.up.sql
  • web/handler.go
  • web/source.go
  • web/templates/source-overview.html

Comment thread docs/src/content/docs/guides/authentication.md Outdated
Comment thread internal/inboundauth/presets.go
Comment thread internal/inboundauth/verify.go
Comment thread internal/store/source_auth_integration_test.go Outdated
Comment thread web/source.go Outdated
…ssets

Adds a third binary, cmd/mcp, a stdio Model Context Protocol server exposing
read-only list_sources, list_actions, and list_deliveries tools backed by the
existing store (github.com/modelcontextprotocol/go-sdk).

Also included:
- docker-compose: add Grafana service (promotes prometheus/client_golang to a
  direct dependency)
- web/static: vendor htmx + Monaco editor assets for the script editor UI
- Makefile: run-mcp target and bin/mcp build
- docs: MCP server guide, tour page, local-dev quickstart section; CLAUDE.md
  updated to three binaries; astro nav entries
- go.mod: bump to go 1.25, add go-sdk and supporting deps
- .gitignore: ignore local .superpowers/ tooling state

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

53-53: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the Go minimum to 1.25
README.md:29, README.md:53, and docs/src/content/docs/getting-started/quickstart.md:49 still say Go 1.24/1.24+, which no longer matches go.mod (go 1.25.0). Keep the published docs in sync.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 53, Update the documented minimum Go version from
1.24/1.24+ to 1.25/1.25+ in the README prerequisite text and the corresponding
getting-started quickstart content, keeping all published references consistent
with the go.mod requirement.

Source: Coding guidelines

🧹 Nitpick comments (2)
docker-compose.yml (1)

32-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Hardcoded Grafana admin credentials.

admin/admin are Grafana's own defaults, so functionally nothing changes, but baking them into source control on a host-published port (3000:3000) makes it easy for this dev-oriented file to be reused as-is in a more exposed environment. Consider making these overridable via .env/shell vars with the same defaults.

🔒️ Suggested fix
     environment:
-      GF_SECURITY_ADMIN_USER: admin
-      GF_SECURITY_ADMIN_PASSWORD: admin
+      GF_SECURITY_ADMIN_USER: ${GF_SECURITY_ADMIN_USER:-admin}
+      GF_SECURITY_ADMIN_PASSWORD: ${GF_SECURITY_ADMIN_PASSWORD:-admin}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docker-compose.yml` around lines 32 - 34, Update the Grafana environment
entries in the docker-compose configuration to read GF_SECURITY_ADMIN_USER and
GF_SECURITY_ADMIN_PASSWORD from .env or shell variables, while retaining admin
as the local development default. Keep the existing Grafana service and port
mapping unchanged.
cmd/mcp/main.go (1)

59-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Mark the tools read-only

Add Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true} to these mcp.Tool definitions so MCP hosts can treat the calls as safe to auto-invoke.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/mcp/main.go` around lines 59 - 105, Add Annotations with ReadOnlyHint set
to true to the mcp.Tool definitions for list_sources, list_actions, and
list_deliveries, preserving their existing names, descriptions, and handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CLAUDE.md`:
- Around line 30-34: Update the build command documentation in CLAUDE.md to
include bin/mcp alongside bin/api and bin/worker, matching the binaries produced
by the Makefile target. Keep the existing command description otherwise
unchanged.

In `@go.mod`:
- Line 52: Upgrade the github.com/quic-go/quic-go dependency referenced by the
go.mod module requirements to a release that resolves the reachable HTTP/3/QPACK
finding, and refresh the module checksum data as needed. Verify the resulting
dependency graph through github.com/gin-gonic/gin/http3; retain the
golang.org/x/crypto finding only if the scanner confirms an actually used
package.

In `@internal/testutil/testutil.go`:
- Around line 23-30: Update the JSON comparison logic in JSONEqual to decode
numeric values without converting them to float64, using Decoder.UseNumber() or
an equivalent numeric-aware comparator. Preserve exact distinctions between
large integers while retaining the existing validation failures and
deep-equality behavior.

---

Outside diff comments:
In `@README.md`:
- Line 53: Update the documented minimum Go version from 1.24/1.24+ to
1.25/1.25+ in the README prerequisite text and the corresponding getting-started
quickstart content, keeping all published references consistent with the go.mod
requirement.

---

Nitpick comments:
In `@cmd/mcp/main.go`:
- Around line 59-105: Add Annotations with ReadOnlyHint set to true to the
mcp.Tool definitions for list_sources, list_actions, and list_deliveries,
preserving their existing names, descriptions, and handlers.

In `@docker-compose.yml`:
- Around line 32-34: Update the Grafana environment entries in the
docker-compose configuration to read GF_SECURITY_ADMIN_USER and
GF_SECURITY_ADMIN_PASSWORD from .env or shell variables, while retaining admin
as the local development default. Keep the existing Grafana service and port
mapping unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c9f7abd8-a5e2-4f4f-aa3e-ab3a36e9fbf0

📥 Commits

Reviewing files that changed from the base of the PR and between dad5918 and c82259d.

⛔ Files ignored due to path filters (3)
  • go.sum is excluded by !**/*.sum
  • web/static/htmx.min.js is excluded by !**/*.min.js
  • web/static/vs/base/browser/ui/codicons/codicon/codicon.ttf is excluded by !**/*.ttf
📒 Files selected for processing (29)
  • .gitignore
  • CLAUDE.md
  • Makefile
  • README.md
  • cmd/mcp/main.go
  • docker-compose.yml
  • docs/astro.config.mjs
  • docs/src/content/docs/getting-started/quickstart.md
  • docs/src/content/docs/guides/authentication.md
  • docs/src/content/docs/guides/mcp.md
  • docs/src/content/docs/tour.md
  • docs/superpowers/plans/2026-07-10-incoming-webhook-authentication.md
  • go.mod
  • internal/inboundauth/presets.go
  • internal/inboundauth/verify.go
  • internal/inboundauth/verify_test.go
  • internal/store/source_auth_integration_test.go
  • internal/store/store_integration_test.go
  • internal/testutil/testutil.go
  • internal/worker/worker_integration_test.go
  • web/source.go
  • web/static/vs/base/worker/workerMain.js
  • web/static/vs/basic-languages/javascript/javascript.js
  • web/static/vs/basic-languages/typescript/typescript.js
  • web/static/vs/editor/editor.main.css
  • web/static/vs/editor/editor.main.js
  • web/static/vs/language/typescript/tsMode.js
  • web/static/vs/language/typescript/tsWorker.js
  • web/static/vs/loader.js
🚧 Files skipped from review as they are similar to previous changes (7)
  • internal/store/source_auth_integration_test.go
  • internal/inboundauth/presets.go
  • internal/inboundauth/verify.go
  • docs/src/content/docs/guides/authentication.md
  • web/source.go
  • internal/inboundauth/verify_test.go
  • docs/superpowers/plans/2026-07-10-incoming-webhook-authentication.md

Comment thread CLAUDE.md
Comment on lines +30 to +34
Three binaries sharing the same internal packages:

- **`cmd/api`** — HTTP server (Gin). Ingests webhooks at `POST /webhooks/:sourceSlug`, serves REST API under `/api/`, and a web UI. Supports `--migrate` and `--worker` flags (in-process worker).
- **`cmd/worker`** — Standalone fan-out worker. Reads from Redis Stream `deliveries` (consumer group `fanout-workers`), dispatches to actions with exponential backoff retry.
- **`cmd/mcp`** — Model Context Protocol server (stdio). Exposes read-only `list_sources`, `list_actions`, `list_deliveries` tools backed by the same store. Built on `github.com/modelcontextprotocol/go-sdk`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n 'make build|bin/(api|worker|mcp)' README.md CLAUDE.md Makefile

Repository: zachbroad/nitrohook

Length of output: 530


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo '--- CLAUDE.md ---'
sed -n '1,40p' CLAUDE.md
echo
echo '--- README.md (around build docs) ---'
sed -n '120,145p' README.md
echo
echo '--- Makefile (build target) ---'
sed -n '30,40p' Makefile

Repository: zachbroad/nitrohook

Length of output: 3037


Update the make build note to include bin/mcp

Makefile builds bin/api, bin/worker, and bin/mcp, so the CLAUDE.md build command comment is stale. Align it with the actual target output.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 30 - 34, Update the build command documentation in
CLAUDE.md to include bin/mcp alongside bin/api and bin/worker, matching the
binaries produced by the Makefile target. Keep the existing command description
otherwise unchanged.

Comment thread go.mod
github.com/prometheus/common v0.66.1 // indirect
github.com/prometheus/procfs v0.16.1 // indirect
github.com/quic-go/qpack v0.5.1 // indirect
github.com/quic-go/quic-go v0.54.0 // indirect

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n --glob '*.go' \
  'github\.com/quic-go/quic-go|golang\.org/x/crypto/(ssh|ssh/agent|ssh/knownhosts)' .

rg -n \
  'github\.com/quic-go/quic-go|golang\.org/x/crypto' \
  go.mod go.sum

Repository: zachbroad/nitrohook

Length of output: 157


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '\n== go.mod ==\n'
cat -n go.mod | sed -n '1,140p'

printf '\n== go.sum matches ==\n'
rg -n 'github.com/quic-go/quic-go|golang.org/x/crypto|golang.org/x/net|golang.org/x/text' go.sum go.mod || true

printf '\n== repo-wide matches ==\n'
rg -n --glob '*.go' 'github\.com/quic-go/quic-go|golang\.org/x/crypto/(ssh|ssh/agent|ssh/knownhosts)|golang\.org/x/crypto' . || true

Repository: zachbroad/nitrohook

Length of output: 4754


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '\n== why quic-go ==\n'
go mod why -m github.com/quic-go/quic-go || true

printf '\n== why x/crypto ==\n'
go mod why -m golang.org/x/crypto || true

printf '\n== module graph edges mentioning them ==\n'
go mod graph | rg 'github.com/quic-go/quic-go|golang.org/x/crypto' || true

Repository: zachbroad/nitrohook

Length of output: 5705


Upgrade the reachable HTTP/3 dependency before release.

  • github.com/quic-go/quic-go v0.54.0 is pulled into cmd/api through github.com/gin-gonic/gin/http3, so the HTTP/3/QPACK finding is in the built graph.
  • golang.org/x/crypto v0.48.0 is present through github.com/go-playground/validator/v10golang.org/x/crypto/sha3, not ssh; only keep that finding if the scanner targets a package actually used here.
🧰 Tools
🪛 OSV Scanner (2.4.0)

[HIGH] 52-52: github.com/quic-go/quic-go 0.54.0: Panic occurs when queuing undecryptable packets after handshake completion in github.com/quic-go/quic-go

(GO-2025-4017)


[HIGH] 52-52: github.com/quic-go/quic-go 0.54.0: HTTP/3 QPACK Header Expansion DoS in github.com/quic-go/quic-go

(GO-2025-4233)


[HIGH] 52-52: github.com/quic-go/quic-go 0.54.0: HTTP/3 QPACK Trailer Expansion Memory Exhaustion in github.com/quic-go/quic-go

(GO-2026-5676)


[HIGH] 52-52: github.com/quic-go/quic-go 0.54.0: quic-go: Panic occurs when queuing undecryptable packets after handshake completion

(GHSA-47m2-4cr7-mhcw)


[HIGH] 52-52: github.com/quic-go/quic-go 0.54.0: quic-go HTTP/3 QPACK Header Expansion DoS

(GHSA-g754-hx8w-x2g6)


[HIGH] 52-52: github.com/quic-go/quic-go 0.54.0: quic-go: HTTP/3 QPACK Trailer Expansion Memory Exhaustion

(GHSA-vvgj-x9jq-8cj9)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go.mod` at line 52, Upgrade the github.com/quic-go/quic-go dependency
referenced by the go.mod module requirements to a release that resolves the
reachable HTTP/3/QPACK finding, and refresh the module checksum data as needed.
Verify the resulting dependency graph through github.com/gin-gonic/gin/http3;
retain the golang.org/x/crypto finding only if the scanner confirms an actually
used package.

Source: Linters/SAST tools

Comment on lines +23 to +30
var av, bv any
if err := json.Unmarshal(a, &av); err != nil {
t.Fatalf("JSONEqual: first argument is not valid JSON (%q): %v", a, err)
}
if err := json.Unmarshal(b, &bv); err != nil {
t.Fatalf("JSONEqual: second argument is not valid JSON (%q): %v", b, err)
}
return reflect.DeepEqual(av, bv)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/sh
set -eu
stdlib="$(go env GOROOT)/src/encoding/json"
go doc encoding/json.Decoder.UseNumber
rg -n 'float64|UseNumber|convertNumber' "$stdlib"

Repository: zachbroad/nitrohook

Length of output: 38492


🏁 Script executed:

#!/bin/sh
set -eu
cat >/tmp/json_equal_probe.go <<'PY'
package main

import (
	"encoding/json"
	"fmt"
	"reflect"
)

func decode(s string) any {
	var v any
	if err := json.Unmarshal([]byte(s), &v); err != nil {
		panic(err)
	}
	return v
}

func main() {
	cases := [][2]string{
		{`9007199254740992`, `9007199254740993`},
		{`9007199254740992`, `9007199254740994`},
		{`1`, `1.0`},
		{`{"n":9007199254740992}`, `{"n":9007199254740993}`},
	}
	for _, c := range cases {
		a := decode(c[0])
		b := decode(c[1])
		fmt.Printf("%s vs %s\n  a=%T %#v\n  b=%T %#v\n  equal=%v\n\n", c[0], c[1], a, a, b, b, reflect.DeepEqual(a, b))
	}
}
PY
go run /tmp/json_equal_probe.go

Repository: zachbroad/nitrohook

Length of output: 668


Preserve JSON numbers during comparison. json.Unmarshal into any turns numbers into float64, so distinct integers above 2^53 can compare equal and hide payload corruption. Decode with Decoder.UseNumber() or another numeric-aware comparator.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/testutil/testutil.go` around lines 23 - 30, Update the JSON
comparison logic in JSONEqual to decode numeric values without converting them
to float64, using Decoder.UseNumber() or an equivalent numeric-aware comparator.
Preserve exact distinctions between large integers while retaining the existing
validation failures and deep-equality behavior.

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.

1 participant