Skip to content

fix(security): sanitize agent-card topology, anti-spoof login rate-limit peer IP, and add 429 Retry-After (#S1 #S2 #S4) - #11418

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
HouMinXi:fix/security-s1-s2-s4
Aug 24, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.50from
HouMinXi:fix/security-s1-s2-s4

Conversation

@HouMinXi

Copy link
Copy Markdown
Contributor

Summary

  • S1: Key loginGuard on authenticated/stamped peer IP rather than client-controlled XFF to prevent login rate-limit bypass via forged XFF rotation.
  • S2: Strip http://localhost:20128 topology disclosure from agent-card.json and agent.json by dynamically resolving the public request origin when OMNIROUTE_BASE_URL is unset.
  • S4: Add standard Retry-After header on login 429 lockouts.
  • Adds comprehensive unit tests in tests/unit/security-s1-s2-s4.test.ts.

Verification

…mit peer IP, and add 429 Retry-After (#S1 #S2 #S4)

- S1: Key loginGuard on authenticated/stamped peer IP rather than client-controlled XFF to prevent rate-limit bypass via forged XFF rotation
- S2: Strip localhost:20128 topology leak from agent-card.json and agent.json by dynamically resolving the public request origin when OMNIROUTE_BASE_URL is unset
- S4: Add standard Retry-After header on login 429 lockouts
- Adds comprehensive unit tests covering all three vectors in tests/unit/security-s1-s2-s4.test.ts

Signed-off-by: Minxi Hou <[EMAIL_REDACTED]>
@HouMinXi
HouMinXi requested a review from diegosouzapw as a code owner August 24, 2026 17:34
…STAMP_TOKEN

P1 (Findings 1, 6, 8): login/route.ts
- Import AUTHZ_HEADER_TRUSTED_PEER_IP from @/server/authz/headers
- Remove local const TRUSTED_PEER_IP_HEADER
- Gate trusted-peer-IP header on OMNIROUTE_PEER_STAMP_TOKEN env var:
  without the token, the header is ignored and auditContext.ipAddress
  is used instead (prevents IP spoofing by untrusted peers)
- Add NextRequest type to POST(request)
- Remove optional chaining on headers.get

P2 (Finding 2): loop counts in tests
- Replace hardcoded 10 with FAILURE_THRESHOLD + 1 in all three S1 tests

P2 (Finding 3): spoofed-header rejection test
- TDD verified: RED without fix (spoofed IPs each get their own bucket),
  GREEN with fix (all requests share the XFF IP bucket)
- Uses different spoofed IP per iteration to detect unconditional trust bug

P3 (Finding 4, 5): shared getBaseUrl()
- Extract getBaseUrl() to src/lib/wellKnown.ts
- Both agent route files use the import instead of local function
- Rename BASE_URL to baseUrl

P3 (Misc): .gitignore includes .code-forge/

Signed-off-by: Houminxi <houminxi@users.noreply.github.com>
Signed-off-by: Minxi Hou <houminxi@gmail.com>
diegosouzapw pushed a commit that referenced this pull request Aug 24, 2026
)

Validado em lote combinado (batch-0824g, junto de #11388/#11397/#11415/#11418) contra o tip de release/v3.8.50: typecheck:core limpo, file-size/changelog/complexity/cognitive-complexity OK, 62/62 testes focados passando.

Diagnóstico correto e bem documentado: a falha do nightly Node 26 era um teste que sorteia um número e depende do resultado, não uma quebra de compatibilidade. Comportamento de produção inalterado (a janela de jitter continua aleatória; só o teste ganhou controle sobre ela). Obrigado pela investigação detalhada!
@diegosouzapw
diegosouzapw merged commit 59dccdd into cheaper-inference:release/v3.8.50 Aug 24, 2026
9 of 16 checks passed
diegosouzapw added a commit that referenced this pull request Aug 25, 2026
PR #11418 (S2 topology sanitisation) removed the hardcoded
localhost:20128 from both well-known agent-card routes and made them
derive the base URL from `request.nextUrl.origin` via
`getBaseUrl(request)` (src/lib/wellKnown.ts). That changed the handler
contract: `GET` now requires the request Next.js always passes it.

Three sibling test files were never aligned and still invoked the
handler as a bare `GET()`, so every case blew up with
`TypeError: Cannot read properties of undefined (reading nextUrl)`
before reaching a single assertion — 8 base-reds from one moved
contract, not from a skill-count drift.

Align the callers to the shipped contract with a local
`makeCardRequest()` helper mirroring tests/unit/security-s1-s2-s4.test.ts
(a Request with a defined `nextUrl`). No assertion was removed,
loosened or skipped; the assert counts are unchanged and the cases now
actually execute.

Refs #11418
@HouMinXi
HouMinXi deleted the fix/security-s1-s2-s4 branch September 16, 2026 14:09
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…aper-inference#11380)

Validado em lote combinado (batch-0824g, junto de cheaper-inference#11388/cheaper-inference#11397/cheaper-inference#11415/cheaper-inference#11418) contra o tip de release/v3.8.50: typecheck:core limpo, file-size/changelog/complexity/cognitive-complexity OK, 62/62 testes focados passando.

Diagnóstico correto e bem documentado: a falha do nightly Node 26 era um teste que sorteia um número e depende do resultado, não uma quebra de compatibilidade. Comportamento de produção inalterado (a janela de jitter continua aleatória; só o teste ganhou controle sobre ela). Obrigado pela investigação detalhada!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…mit peer IP, and add 429 Retry-After (#S1 #S2 #S4) (cheaper-inference#11418)

Validado em lote combinado (batch-0824g) contra o tip de release/v3.8.50: typecheck:core limpo, gates estáticos OK, 62/62 testes focados passando (S1/S2/S4, tests/unit/security-s1-s2-s4.test.ts, 9/9).

Boa integração com o padrão já existente de peer IP stamped por HMAC (resolveStampedPeer/OMNIROUTE_PEER_STAMP_TOKEN) — reusa em vez de reimplementar, e o header confiável só é honrado quando o stamp token está configurado. S2 remove corretamente a disclosure de topologia hardcoded do agent-card. Obrigado pela contribuição!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
PR cheaper-inference#11418 (S2 topology sanitisation) removed the hardcoded
localhost:20128 from both well-known agent-card routes and made them
derive the base URL from `request.nextUrl.origin` via
`getBaseUrl(request)` (src/lib/wellKnown.ts). That changed the handler
contract: `GET` now requires the request Next.js always passes it.

Three sibling test files were never aligned and still invoked the
handler as a bare `GET()`, so every case blew up with
`TypeError: Cannot read properties of undefined (reading nextUrl)`
before reaching a single assertion — 8 base-reds from one moved
contract, not from a skill-count drift.

Align the callers to the shipped contract with a local
`makeCardRequest()` helper mirroring tests/unit/security-s1-s2-s4.test.ts
(a Request with a defined `nextUrl`). No assertion was removed,
loosened or skipped; the assert counts are unchanged and the cases now
actually execute.

Refs cheaper-inference#11418
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