Repository navigation
feat: stateless web search capability tool (web-search@1 reference implementation) - #1
Conversation
…mplementation
hadrontool-websearch is the reference implementation of the versioned,
provider-agnostic web-search@1 contract (platform specs cor:web:020*), the
agent-facing search capability whose core-side spine landed in hadron-server#648.
A separate stateless tool from hadrontool-webfetch (not a mode of fetch):
- POST /ops/search speaks the normalized envelope {version?, provider?, query,
count?, freshness?, locale?, providerOptions?, auth?} → {ok, results:
[{title,url,snippet,rank}], contractVersion, source:'external'}.
- Fixed provider allowlist (src/providers.ts: brave default + bing) — a request
names a slug, never a URL, so the egress is deliberately GUARD-FREE: no SSRF
guard, no connection pinning (spec cor:web:020:02).
- Provider credential arrives inline per request via auth (bearer/basic/header),
core-sourced, stored nowhere, attached only to the provider request, never in
a URL or a log.
- Stable typed error catalog (validation_error, provider_not_configured,
provider_unauthorized, provider_rejected, provider_rate_limited,
upstream_unreachable, upstream_timeout) — passed through by core verbatim.
- Untrusted provider output degrades to [] on malformed/unexpected shapes.
Copies the hadrontool-webfetch stateless skeleton (config/errors/auth/routes/
Dockerfile), including the tsconfig.build.json COPY fix (hadrontool-webfetch#3).
36 tests over a fake fetch seam; typecheck + build clean; live smoke-tested.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces hadrontool-websearch, a stateless microservice that queries external search providers (Brave and Bing) and returns a normalized, versioned result envelope. The implementation is clean, well-tested, and adheres to security principles by keeping credentials out of logs and configuration. However, two security improvements are recommended: first, update the safeEqual function in src/middleware/auth.ts to hash tokens before comparison to prevent leaking the secret token's length via a timing side-channel; second, strengthen URL validation in src/search.ts to ensure only valid absolute HTTP/HTTPS URLs are returned, preventing potential downstream vulnerabilities.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import { timingSafeEqual } from 'node:crypto'; | ||
|
|
||
| function safeEqual(a: string, b: string): boolean { | ||
| const ab = Buffer.from(a); | ||
| const bb = Buffer.from(b); | ||
| if (ab.length !== bb.length) return false; | ||
| return timingSafeEqual(ab, bb); | ||
| } |
There was a problem hiding this comment.
The safeEqual function leaks the byte length of the secret serviceToken because it returns false immediately if the lengths of the two buffers do not match (ab.length !== bb.length). This introduces a timing side-channel that allows an attacker to determine the length of the secret token.
To prevent this length leak while remaining timing-safe, you can hash both strings using a fixed-size hash (such as SHA-256) before comparing them with timingSafeEqual. Since the SHA-256 hashes will always have the same length (32 bytes), you can safely compare them without leaking the original token's length.
import { createHash, timingSafeEqual } from 'node:crypto';
function safeEqual(a: string, b: string): boolean {
const hashA = createHash('sha256').update(a).digest();
const hashB = createHash('sha256').update(b).digest();
return timingSafeEqual(hashA, hashB);
}| const results: SearchResult[] = raw | ||
| .filter((r) => r.url) // a reference without a URL is not usable | ||
| .slice(0, count) | ||
| .map((r, i) => ({ title: r.title, url: r.url, snippet: r.snippet, rank: i + 1 })); |
There was a problem hiding this comment.
The search results are filtered only by checking if r.url is truthy. However, if an upstream provider returns a relative URL or a URL with an unexpected/dangerous protocol (such as javascript:, data:, or file:), it will be passed through to the caller. This can lead to security vulnerabilities (like SSRF or XSS) or runtime errors when the caller attempts to use the URL.
To enforce defensive programming and ensure only valid absolute HTTP/HTTPS URLs are returned, validate the URL protocol using new URL() before returning the results.
const results: SearchResult[] = raw
.filter((r) => {
try {
const parsed = new URL(r.url);
return parsed.protocol === 'http:' || parsed.protocol === 'https:';
} catch {
return false;
}
})
.slice(0, count)
.map((r, i) => ({ title: r.title, url: r.url, snippet: r.snippet, rank: i + 1 }));There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09324a5bbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| let res: SearchFetchResponse; | ||
| try { | ||
| res = await deps.fetchImpl(url, { method: 'GET', headers, signal: controller.signal }); |
There was a problem hiding this comment.
Reject redirects before leaving the allowlist
In production, undici/fetch follows HTTP redirects by default; because this call does not set redirect: 'error'/manual or validate the final URL, a 30x from an allowlisted provider sends the tool to the redirected Location while it is still treated as fixed-allowlist egress. That bypasses the no-SSRF/no-pinning invariant for any provider redirect or incident, so redirects should be failed or the redirected host re-checked before following.
Useful? React with 👍 / 👎.
|
|
||
| let text: string; | ||
| try { | ||
| text = await res.text(); |
There was a problem hiding this comment.
Enforce the response byte cap while streaming
When the provider omits Content-Length or uses chunked transfer, res.text() buffers the entire response before the MAX_RESPONSE_BYTES check runs. In that scenario a bad upstream response can make this stateless service allocate an arbitrarily large body and potentially OOM despite the intended cap; read the body as a stream and abort once the byte budget is exceeded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Introduces hadrontool-websearch, a stateless Express-based capability tool that implements the provider-agnostic web-search@1 contract by querying an allowlisted external search provider and returning a normalized, ranked result envelope.
Changes:
- Adds a provider registry (
bravedefault, plusbing) and a search engine with a fakeable fetch seam and defensive response normalization. - Implements the
POST /ops/searchoperation surface (schema-validated) with bearer-gated ops routing plus open health endpoints. - Adds typed error catalog + inline credential channel support, along with a Vitest/Supertest test suite and build/CI/Docker packaging.
Reviewed changes
Copilot reviewed 24 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds Vitest configuration for node-based unit/integration tests. |
| tsconfig.json | Defines strict NodeNext TypeScript compilation settings. |
| tsconfig.build.json | Excludes test files from production builds. |
| src/search.ts | Implements the allowlist-based outbound search request and normalization logic. |
| src/search.test.ts | Tests search normalization, clamping, credential attachment, and error mapping. |
| src/routes/ops.ts | Adds /ops/:operation router with typed error handling and logging. |
| src/routes/ops.test.ts | Supertest coverage for /ops/search, versioning, strict schema, and auth gate. |
| src/routes/health.ts | Adds open /healthz and /readyz endpoints. |
| src/providers.ts | Defines the provider allowlist and provider-specific URL/response adapters. |
| src/providers.test.ts | Tests provider selection, URL mapping, and defensive parsing. |
| src/ops/index.ts | Registers the search operation with a single strict zod schema and version check. |
| src/middleware/auth.ts | Adds timing-safe bearer-token middleware for ops plane. |
| src/logger.ts | Adds minimal structured JSON logger with “no sensitive logging” guidance. |
| src/index.ts | Bootstraps server startup and logs basic runtime configuration. |
| src/errors.ts | Defines the stable error catalog and zod-to-validation error mapping. |
| src/config.ts | Validates environment configuration and enforces prod auth-token requirement. |
| src/auth.ts | Implements provider-agnostic inline credential schema and header construction. |
| src/auth.test.ts | Tests auth header construction and schema validation. |
| src/app.ts | Wires Express middleware, routes, auth gate, and global error handling. |
| README.md | Documents the contract, endpoints, provider model, and local commands. |
| package.json | Declares runtime/dependencies, scripts, and Node engine requirement. |
| package-lock.json | Locks dependency graph for reproducible installs. |
| LICENSE | Adds MIT license text. |
| Dockerfile | Builds a production container image (with Doppler runtime secret injection). |
| CLAUDE.md | Adds agent-facing dev/architecture notes for the repository. |
| .gitignore | Ignores build artifacts, deps, env files, logs. |
| .github/workflows/ci.yml | Adds CI pipeline for install, typecheck, and tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Body-parser raise (e.g. payload too large / malformed JSON) carries a status. | ||
| const status = (err as { status?: number })?.status ?? (err as { statusCode?: number })?.statusCode; | ||
| if (typeof status === 'number') { | ||
| res.status(status).json({ error: 'bad_request', message: (err as Error).message }); | ||
| return; | ||
| } |
|
From a security review: ⏺ Security Review — PR #1 (feat/initial-websearch-tool) No high-confidence exploitable vulnerabilities found. Scope Reviewed the 27 files added in this PR (4,797 additions), focusing on src/app.ts, src/auth.ts, src/config.ts, src/errors.ts, src/index.ts, src/logger.ts, src/middleware/auth.ts, src/ops/index.ts, Findings considered and rejected Two candidate findings surfaced during the initial pass and were rejected by parallel false-positive filters (both scored 2/10 confidence):
Categories verified clean
|
Review fixes for PR #1: - search: refuse provider redirects instead of following them. undici's fetch defaults to redirect:'follow', so a 30x from an allowlisted provider would leave the fixed allowlist — the one thing the guard-free egress must never do. Now redirect:'manual' + a 3xx is a typed upstream_unreachable (Codex P2). - search: enforce the response byte cap while STREAMING (readCapped over the body reader) instead of buffering via res.text(), closing the OOM window on a Content-Length-less / chunked response (Codex P2). - search: drop result references that are not absolute http(s) URLs — reject relative URLs and dangerous schemes (javascript:/data:/file:) a provider might return (Gemini). - auth: compare the bearer token via fixed-size SHA-256 digests so the timing- safe compare leaks nothing about the token length (Gemini). - app: map body-parser errors (malformed JSON / oversized body) to the in-catalog validation_error instead of an ad-hoc bad_request (Copilot). - providers: str() now trims, matching its doc and keeping stray whitespace out of normalized titles/snippets (Copilot). - ci: pin node-version to the engines floor 22.12.0 (Copilot). Tests: 39 pass (added redirect-refusal, non-http-URL filtering, malformed-JSON → validation_error). typecheck + build clean; live smoke re-verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
All seven review comments addressed in the latest commit:
39 tests pass; typecheck + build clean; live smoke re-verified. |
Summary
hadrontool-websearch— the reference implementation of the versioned,provider-agnostic
web-search@1contract (platform specscor:web:020*),the agent-facing web search capability whose core-side spine merged in
hadron-server#648. A separate stateless tool from
hadrontool-webfetch(not a mode of fetch), copied from its stateless skeleton.
Design: hadron-server#647. All authorization stays in core (
tool.web_search);this tool executes searches only.
What's here
src/providers.tsbrave(default) +bing. A request names a slug, never a URL.src/search.ts{title,url,snippet,rank}. Guard-free — no SSRF guard, no connection pinning (cor:web:020:02), because the destination is never caller-supplied. Injectable fetch seam.src/auth.tsauthorization/cookiecan't be set via theheadershape.src/ops/index.tsPOST /ops/search, one zod schema viadefineOp, contract-version check.src/errors.tssrc/routes/ops(bearer-gated),health(open).Contract conformance (matches merged
webSearchClient.ts){version?, provider?, query, count?, freshness?, locale?, providerOptions?, auth?}→ response{ok, results:[{title,url,snippet,rank}], contractVersion:"web-search@1", source:"external"}.cor:web:020:00— separate tool; results taggedsource:'external'; search returns references and never fetches them.cor:web:020:01— versioned provider-agnostic envelope; callers never branch on provider identity;providerOptionsaccepted and safely ignored.cor:web:020:02— destination is a provider slug from a fixed allowlist (materially lower SSRF risk → no pinning); credential inline, core-sourced, off the URL, stored nowhere.validation_error,provider_not_configured,provider_unauthorized,provider_rejected,provider_rate_limited,upstream_unreachable,upstream_timeout.Verification
npm run typecheckclean; 36 tests pass (real HTTP via supertest over a fake fetch seam — no network).npm run buildclean, 0 test files indist/; reproduced the Dockerfile COPY-set in a clean dir → build exit 0 (carries thetsconfig.build.jsonCOPY fix from hadrontool-webfetch#3)./healthz, bearer gate (401),validation_error(400),provider_not_configured(503), unsupported-version rejection — all emit the correct{error,message,…}envelope.Deferred (mirrors core #647 scope)
cor:web:020:02in full) — today the credential is a single inline provider sourced by core; no org-scoped trust tiers here.cor:web:020:01§4) — theweb-search@1constant is in place; the canonical-inputs→envelope set that would formally gate interchangeable tools is not.providerOptionsis accepted but no adapter consumes it yet.Deploy (after merge): follows
ops:alpha:deploy-capability-tool— boots staged untilWEBSEARCH_TOOL_TOKEN+ a provider key are wired in core.Refs: hadron-server#647 · hadron-server#648 · specs
cor:web:020*🤖 Generated with Claude Code