Skip to content

fix(api): reject unsupported health methods - #100

Open
seonghobae wants to merge 4 commits into
mainfrom
fix/health-method-contract-on-main
Open

fix(api): reject unsupported health methods#100
seonghobae wants to merge 4 commits into
mainfrom
fix/health-method-contract-on-main

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Purpose

Make runtime liveness behavior match the public API contract: /health must not return a false 200 liveness success for unsupported HTTP methods.

Exact source identity

  • protected base: main at c85d710804139c0697d7ef8fa47d02b1389e6d84;
  • head branch: fix/health-method-contract-on-main;
  • RED regression head before the runtime repair: afd829e80bdb1e52ca543bdd2b135cf3f0722a22;
  • first GREEN runtime head: f59951e13da25384244e337c02c4e860a9932763;
  • current exact head after protocol-test hardening: c3b767c1c5b45801151d8a72475f815ff458ac8e;
  • the diff remains bounded to src/index.ts plus test/health-method-contract.test.ts.

RCA and test-first proof

The first failing boundary was the deployed runtime route itself: the /health branch returned success solely from pathname matching and never validated the request method. Exact-head application CI run 31398252729 checked out afd829e80bdb1e52ca543bdd2b135cf3f0722a22 and failed only the new health-method regression: unsupported methods returned 200 instead of 405; 647 existing tests passed.

The narrow production repair reuses Noema's existing ApiError and standard response/logging path before liveness success. Unsupported methods now return 405, Allow: GET, ERR_VALIDATION_INPUT, and the normal diagnostic/security headers. GET /health remains unchanged.

A follow-up executable contract on current head c3b767c1c5b45801151d8a72475f815ff458ac8e expands the unsupported-method boundary to HEAD, POST, PUT, PATCH, DELETE, and OPTIONS, and pins content-type, cache-control: no-store, pragma: no-cache, x-content-type-options: nosniff, trace presence, and body/header trace-ID consistency. This matches the protected API specification, which defines only GET /health.

Current exact-head evidence

For unchanged head c3b767c1c5b45801151d8a72475f815ff458ac8e:

  • application ci run 31402062203: terminal success; verify job 93499202012 completed exact checkout, toolchain validation, install, and release verify successfully;
  • reviewer-ci run 31402062029: terminal success; reviewer job 93499201726 completed exact checkout, 100% reviewer line/branch coverage, 100% docstring gate, immutable sandbox-image scanning, and real no-network sandbox smoke successfully;
  • central Security Scan run 31402061961: terminal success under its own scanner/revision semantics;
  • current formal GitHub reviews: 0;
  • current inline review threads: 0;
  • GitHub reports the PR mergeable and Ready for review.

Merge / operational boundary

Technical success is not merge authority. Live enforceable governance under #27 and an eligible qualifying independent non-author formal-review path under #29 remain separate fail-closed gates where required. No self-approval, protection weakening, synthetic status, release, deployment, outbound-license decision, or acquisition-readiness claim is made.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c2868f86-d91f-4f0b-8b9d-573006ea3953

📥 Commits

Reviewing files that changed from the base of the PR and between c85d710 and c3b767c.

📒 Files selected for processing (2)
  • src/index.ts
  • test/health-method-contract.test.ts

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.

@seonghobae
seonghobae marked this pull request as ready for review August 10, 2026 15:07

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review unchanged exact head c3b767c1c5b45801151d8a72475f815ff458ac8e against current protected main c85d710804139c0697d7ef8fa47d02b1389e6d84. Focus on whether unsupported /health methods fail before any liveness-success path and whether the 405 protocol/security headers are complete and consistent with runtime/API contracts. Current application CI 31402062203, reviewer-ci 31402062029, and central Security Scan 31402061961 are terminal success, with no current inline threads or formal reviews. Treat this as diagnostic review evidence only; it is not qualifying independent merge approval authority.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='c85d710804139c0697d7ef8fa47d02b1389e6d84'
head='c3b767c1c5b45801151d8a72475f815ff458ac8e'

printf '%s\n' '== Revision identity =='
git rev-parse HEAD
git cat-file -t "$base"
git cat-file -t "$head"
git show -s --format='base: %H%nsubject: %s' "$base"
git show -s --format='head: %H%nsubject: %s' "$head"

printf '%s\n' '== Changed files and diff stat =='
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"

printf '%s\n' '== Full bounded diff =='
git diff --no-ext-diff --unified=80 "$base" "$head" -- src/index.ts test/health-method-contract.test.ts

printf '%s\n' '== Runtime module outline =='
ast-grep outline src/index.ts --items all --view expanded

printf '%s\n' '== Runtime health and shared error-path references =='
rg -n -C 12 -e 'health' -e 'ApiError' -e 'Allow' -e 'ERR_VALIDATION_INPUT' -e 'x-content-type-options' -e 'cache-control' -e 'pragma' src/index.ts

printf '%s\n' '== Health contract test outline and assertions =='
ast-grep outline test/health-method-contract.test.ts --items all --view expanded
cat -n test/health-method-contract.test.ts

Length of output: 35256


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Shared response builders =='
sed -n '120,290p' src/index.ts

printf '%s\n' '== Health API contract references =='
rg -n -i -C 8 \
  -e '(/health|health endpoint|health check|liveness)' \
  -g '!node_modules/**' -g '!dist/**' -g '!build/**' .

printf '%s\n' '== Explicit HTTP-method declarations near health references =='
rg -n -i -C 6 \
  -e '(GET\s+/?health|/health\s*.*GET|method.*GET|allowed_methods.*GET)' \
  -g '!node_modules/**' -g '!dist/**' -g '!build/**' .

Length of output: 125512


@seonghobae Reviewed c3b767c1c5b45801151d8a72475f815ff458ac8e against c85d710804139c0697d7ef8fa47d02b1389e6d84.

I found no issues in the requested scope.

  • In src/index.ts, the /health branch rejects every method except GET before it creates the liveness success response.
  • The rejection enters the standard ApiError handler. It returns 405, ERR_VALIDATION_INPUT, and details.allowed_methods: "GET".
  • The shared response path adds Allow: GET, content-type: application/json; charset=utf-8, cache-control: no-store, pragma: no-cache, x-content-type-options: nosniff, x-trace-id, and x-latency-ms.
  • The body trace_id and the x-trace-id header use the same value.
  • docs/api-spec.md, docs/runtime-readiness.md, and CLAUDE.md specify GET /health. The implementation matches this contract.
  • test/health-method-contract.test.ts covers HEAD, POST, PUT, PATCH, DELETE, and OPTIONS. It pins the 405 status, Allow header, required JSON and security headers, error schema, and trace-ID consistency.

This is diagnostic review evidence only. It is not independent merge approval authority.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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