Skip to content

feat: add ADT proxy server with JSON↔XML conversion#131

Merged
ThePlenkov merged 11 commits into
abapify:mainfrom
ThePlenkov:gt/maple/a80c2beb
Jun 17, 2026
Merged

feat: add ADT proxy server with JSON↔XML conversion#131
ThePlenkov merged 11 commits into
abapify:mainfrom
ThePlenkov:gt/maple/a80c2beb

Conversation

@ThePlenkov

@ThePlenkov ThePlenkov commented Jun 16, 2026

Copy link
Copy Markdown
Member

User description

Summary

This PR adds an ADT proxy server with automatic JSON↔XML conversion to the abapify/adt-cli monorepo.

What's included:

  1. @abapify/speci - createServer() function (packages/speci/src/rest/server/)

    • Framework-agnostic server-side contract route extraction
    • Walks contract trees and extracts route definitions (method, path, body schema, response schemas)
    • Provides URL pattern matching against extracted routes
    • Counterpart to the existing createClient() function
  2. @abapify/adt-proxy - New package (packages/adt-proxy/)

    • ADT proxy server implementation using speci's createServer
    • Automatic JSON→XML conversion for request bodies using schema build() methods
    • Automatic XML→JSON conversion for responses using schema parse() methods
    • Transparent proxying for unmatched routes
    • Configurable target URL, auth, base path, and conversion options
  3. adt proxy CLI command (packages/adt-cli/)

    • New CLI command to start the proxy server
    • Uses current auth session or accepts explicit target URL
    • Supports --port, --host, --target, --base-path, --no-convert options

Architecture

Client (JSON) → Proxy Server → Downstream SAP (XML)
Client (JSON) ← Proxy Server ← Downstream SAP (XML)

The proxy leverages the existing contract definitions and schema parse()/build() methods to automatically convert between JSON and XML. This means:

  • JSON request bodies are converted to XML using the contract's body schema
  • XML responses are converted to JSON using the contract's response schemas
  • Unmatched routes are forwarded as-is (transparent proxy mode)

Usage

# Start proxy using current auth session
npx adt proxy

# Start proxy with explicit target
npx adt proxy --target https://sap-system:8000 --port 3000

# Disable JSON↔XML conversion (pure proxy)
npx adt proxy --no-convert
// Programmatic usage
import { createAdtProxy } from '@abapify/adt-proxy';

const proxy = createAdtProxy({
  targetUrl: 'https://sap-system:8000',
  auth: { username: 'user', password: 'pass', client: '100' },
});

const { port } = await proxy.start();

Tests

  • 13 tests for speci/rest/server (route extraction and matching)
  • 15 tests for adt-proxy converter module
  • All builds pass for affected packages

Next Steps

This PR sets the foundation for:

  • Mock server - The proxy can be extended to serve mock responses based on contract schemas
  • Contract-based routing - Full route matching with path parameter extraction
  • Content negotiation - Automatic Accept/Content-Type header handling

Summary by cubic

Adds @abapify/adt-proxy, a schema‑aware ADT proxy that converts JSON↔XML, plus an adt proxy CLI and createServer() in @abapify/speci for server‑side route extraction. Improves security, routing, and header handling with body-size limits, timeouts, safe base paths, and correct Set-Cookie forwarding.

  • New Features

    • @abapify/adt-proxy: Converts JSON bodies to XML and XML responses to JSON via schema build/parse; forwards unmatched routes; configurable target/auth/base path/conversion/forwarding/maxBodySize (default 10MB); programmatic API; exports jsonToXml/xmlToJson.
    • adt proxy CLI: Start the proxy with --port, --host, --target, --base-path, --no-convert, --no-forward-unknown, --max-body-size, --verbose; uses current auth session when --target is omitted; prints discovered routes; graceful shutdown.
    • @abapify/speci/rest/server: createServer() extracts routes (method, path, body/response schemas) from contracts and provides URL matching for schema‑aware proxying.
  • Bug Fixes

    • Security and reliability: enforce maxBodySize (10MB default) with stream error handling; 30s downstream fetch timeout; register server error before listen(); robust port validation and auth/session guards; CI fallback with --no-cloud when NX_CLOUD_ACCESS_TOKEN is missing.
    • Routing and headers: matched routes always forward with contract requestHeaders even when conversion is disabled; safe base‑path checks with startsWith/slice; simpler path‑param handling via split/join; correct multi‑value headers, including Set-Cookie via getSetCookie() and skipping set-cookie in header iteration to avoid cookie corruption; do not forward client Authorization when proxy auth is set.
    • Content handling and cleanup: case‑insensitive content‑type checks; select response schema by response.status; only mark responses as converted when body changes; add --verbose logging; remove unused fast-xml-parser; reduce matcher complexity in createServer().

Written for commit 937754a. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added a new proxy CLI command to start an ADT proxy server with JSON↔XML conversion.
    • Supports configurable host/port and optional target URL, including base-path handling and forwarding for unknown routes.
    • Automatically derives authentication from the active session (basic credentials or cookie-based auth).
    • Includes graceful shutdown and verbose logging, plus helpful route guidance on startup.
  • Tests
    • Added converter and REST server routing tests to validate JSON/XML conversion and request matching.
  • Chores
    • Improved CI behavior to avoid cloud steps when credentials are not provided.

CodeAnt-AI Description

Add an ADT proxy command that forwards requests to SAP with automatic JSON↔XML conversion

What Changed

  • Added a new adt proxy command to start a local ADT proxy server, with options for target URL, host, port, base path, body size limit, and turning conversion off
  • The proxy now converts JSON request bodies to XML for supported ADT routes, and converts XML responses back to JSON when a matching schema is available
  • Unmatched routes can be forwarded as-is, or returned as 404 when forwarding is disabled
  • Authentication from the current session is now carried through to the downstream SAP system, including cookie-based sessions and explicit target overrides
  • Added route extraction support in speci so server code can match incoming requests against ADT contract paths

Impact

✅ Easier local testing against SAP systems
✅ Fewer manual JSON/XML conversions
✅ Clearer proxy startup and forwarding behavior

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Add speci createServer for extracting route definitions from REST contracts,
and a new adt-proxy package that implements an ADT proxy server with automatic
JSON↔XML conversion using contract schemas. The proxy forwards requests to a
downstream SAP system, converting request bodies from JSON to XML and response
bodies from XML to JSON using the schema parse/build methods.

Also adds 'adt proxy' CLI command for easy proxy server startup.

Changes:
- speci: Add rest/server module with createServer() function
- adt-proxy: New package implementing ADT proxy server
- adt-cli: Add proxy command to CLI
Copilot AI review requested due to automatic review settings June 16, 2026 23:23
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@netlify

netlify Bot commented Jun 16, 2026

Copy link
Copy Markdown

Deploy Preview for adt-cli canceled.

Name Link
🔨 Latest commit 937754a
🔍 Latest deploy log https://app.netlify.com/projects/adt-cli/deploys/6a3258cd66ca480008b5ec4f

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ThePlenkov, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 16 minutes and 45 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

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 credits.

🚦 How do rate limits work?

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

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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10deedb3-db17-4e3d-ae6e-d4b3e37956b1

📥 Commits

Reviewing files that changed from the base of the PR and between d779c4d and 937754a.

📒 Files selected for processing (1)
  • packages/adt-proxy/src/proxy.ts
📝 Walkthrough

Walkthrough

Adds a new @abapify/adt-proxy package implementing an HTTP proxy server with schema-aware JSON↔XML content conversion. Extends @abapify/speci with a createServer utility for server-side REST contract route matching. Exposes the proxy as a new proxy command in adt-cli with configurable auth, host/port, base-path, and conversion options. Also updates CI workflow to conditionally use Nx Cloud based on token availability.

Changes

speci REST Server Extension

Layer / File(s) Summary
Server-side REST types
packages/speci/src/rest/server/types.ts
Introduces ServerRequest, ServerResponse, RouteDefinition, ServerHandler, RouteContext, ServerConfig, and ServerRoutes (with match signature) as framework-agnostic interfaces.
createServer implementation and tests
packages/speci/src/rest/server/create-server.ts, packages/speci/src/rest/server/create-server.test.ts
Walks REST contracts using a sentinel path marker to discover endpoints, converts paths to regexes with named capture groups to build RouteDefinition objects, and implements the match method. Tests cover route extraction, path parameters, query strings, and base-path stripping.
speci server module exports and build wiring
packages/speci/src/rest/server/index.ts, packages/speci/src/rest/index.ts, packages/speci/package.json, packages/speci/tsdown.config.ts
Exposes createServer and server types via a new barrel entry, re-exports from the rest index, adds the ./rest/server subpath export to the package manifest, and adds the build entry to tsdown.

adt-proxy Package and CLI Integration

Layer / File(s) Summary
adt-proxy package scaffold and types
packages/adt-proxy/package.json, packages/adt-proxy/project.json, packages/adt-proxy/tsconfig.json, packages/adt-proxy/tsdown.config.ts, packages/adt-proxy/vitest.config.ts, packages/adt-proxy/src/types.ts
Sets up the new ESM package manifest, Nx project config, TypeScript and build tooling config, and defines AdtProxyConfig, Logger, ProxyRouteContext, and ProxyResult public interfaces.
JSON↔XML converter utilities and tests
packages/adt-proxy/src/converter.ts, packages/adt-proxy/src/converter.test.ts
Implements jsonToXml, xmlToJson, detectContentType, isJsonContentType, and isXmlContentType with fallback-on-failure behavior. Tests cover success paths, error/fallback paths, and content-type classification via header values and content heuristics.
createAdtProxy HTTP proxy server
packages/adt-proxy/src/proxy.ts, packages/adt-proxy/src/index.ts
Implements the proxy server: route matching against ADT contract definitions, downstream URL and header construction (Basic Auth, SAP client, content-type negotiation), forwardRequest via fetch, end-to-end handleRequest with bidirectional conversion, JSON 404/500 error responses, and start()/stop() lifecycle. Barrel entrypoint re-exports everything publicly.
CLI proxy command wiring
packages/adt-cli/src/lib/commands/proxy.ts, packages/adt-cli/src/lib/commands/index.ts, packages/adt-cli/src/lib/cli.ts, packages/adt-cli/package.json
Adds proxyCommand with CLI options for port, host, target, basePath, conversion toggles, and SID; resolves auth via loadAuthSession (basic or cookie); starts the proxy with SIGINT/SIGTERM shutdown handlers. Re-exports through the commands index and registers in createCLI.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as adt-cli proxy command
  participant AdtProxy as createAdtProxy
  participant createServer as createServer (speci)
  participant fetch as fetch (downstream ADT)
  participant converter as JSON/XML converter

  CLI->>AdtProxy: createAdtProxy(config)
  AdtProxy->>createServer: createServer(adtContracts)
  createServer-->>AdtProxy: ServerRoutes (routes + match)
  AdtProxy->>AdtProxy: start() → listen on host:port

  rect rgba(100, 149, 237, 0.5)
    note over CLI,converter: Incoming request flow
    CLI-->>AdtProxy: HTTP request arrives
    AdtProxy->>createServer: match(method, url, basePath)
    createServer-->>AdtProxy: RouteDefinition + params (or null)
    AdtProxy->>converter: jsonToXml(requestBody, requestSchema)
    converter-->>AdtProxy: XML body
    AdtProxy->>fetch: POST/GET to targetUrl (XML body, auth headers)
    fetch-->>AdtProxy: XML response (status, headers, body)
    AdtProxy->>converter: xmlToJson(responseBody, responseSchema)
    converter-->>AdtProxy: JSON body
    AdtProxy-->>CLI: HTTP response (JSON, converted:true)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • abapify/adt-cli#77: Both PRs modify .github/workflows/ci.yml around the nx-cloud/fix-ci CI step logic, with conditional execution based on Nx Cloud token availability.

Poem

🐇 Hopping through XML and JSON streams,
A proxy server built from ADT dreams!
Contracts walk with sentinel in paw,
Regexes capture paths without a flaw.
createAdtProxy — now the bunny's law! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.74% 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
Title check ✅ Passed The title accurately and concisely describes the main feature being added: a new ADT proxy server with JSON↔XML conversion capabilities.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Jun 16, 2026

@amazon-q-developer amazon-q-developer Bot 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.

Summary

This PR adds a well-architected ADT proxy server with JSON↔XML conversion. The implementation is solid overall, but there are 5 critical issues that must be fixed before merge:

Critical Issues Found

  1. ReDoS Vulnerability (2 instances): basePath parameter creates regex injection vulnerability enabling denial of service attacks
  2. Response Conversion Bug: Only HTTP 200 responses are converted; non-200 responses with XML bodies are not converted to JSON
  3. Server Crash Risk: Missing early error handler registration during server startup can cause unhandled rejections
  4. Authentication Failure: Cookie-based auth credentials are never forwarded to downstream requests

Testing Recommendation

After fixes, verify:

  • Proxy handles non-200 responses correctly (201, 204, 4xx, 5xx with XML bodies)
  • Server startup with port conflicts doesn't cause crashes
  • Cookie-based authentication flows work end-to-end
  • ReDoS protection prevents malicious basePath values from hanging the server

All identified issues have blocking severity and require fixes before merge.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Comment thread packages/adt-cli/src/lib/commands/proxy.ts
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/speci/src/rest/server/create-server.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
codescene-delta-analysis[bot]

This comment was marked as outdated.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a new @abapify/adt-proxy package implementing an ADT proxy server with JSON↔XML conversion, integrates a new proxy command into adt-cli, and adds server-side route generation capabilities to @abapify/speci. The code review highlights several important areas for improvement: correctly handling multi-value headers like Set-Cookie using getSetCookie() and updated types; preventing potential request hangs by listening to 'error' events on request streams; avoiding dynamic regex creation from unescaped base paths; dynamically resolving response schemas based on status codes instead of hardcoding 200; and adding defensive checks for session authentication. Additionally, minor improvements are suggested for Commander option parsing and string replacement efficiency.

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.

Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-cli/src/lib/commands/proxy.ts Outdated
Comment thread packages/adt-cli/src/lib/commands/proxy.ts
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/types.ts Outdated
Comment thread packages/speci/src/rest/server/create-server.ts Outdated
Comment thread packages/speci/src/rest/server/create-server.ts Outdated
Comment thread packages/speci/src/rest/server/types.ts Outdated
@codacy-production

codacy-production Bot commented Jun 16, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 critical

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
Security 2 critical

View in Codacy

🟢 Metrics 208 complexity · 6 duplication

Metric Results
Complexity 208
Duplication 6

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Comment thread packages/adt-cli/src/lib/commands/proxy.ts Outdated
Comment thread packages/adt-cli/src/lib/commands/proxy.ts Outdated
Comment thread packages/adt-proxy/src/converter.ts
Comment thread packages/adt-proxy/src/converter.ts Outdated
Comment thread packages/adt-proxy/src/converter.ts Outdated
Address CodeScene complexity review feedback:
- Extract handleMatchedRoute/handleForwardedRequest/sendResponse from handleRequest
- Extract forwardHeaders/addAuthHeaders helpers
- Extract convertRequestBody/convertResponseBody helpers
- Extract tryExtractEndpoint from walkContract to reduce nesting
- Flatten control flow with early returns
codescene-delta-analysis[bot]

This comment was marked as outdated.

Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/speci/src/rest/server/create-server.ts Outdated
Comment thread packages/speci/src/rest/server/create-server.ts Outdated
- Fix ReDoS vulnerability: escape special regex characters in basePath
- Fix cookie auth forwarding: pass cookie/bearer headers via defaultHeaders
  instead of dropping them when cookie-based auth is detected
codescene-delta-analysis[bot]

This comment was marked as outdated.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 10

🤖 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 `@packages/adt-cli/src/lib/commands/proxy.ts`:
- Around line 5-35: The proxyCommand is missing a required --json flag for
machine-readable output. Add a .option() call for the --json flag to the
proxyCommand's option chain (in the same section as the other options like
--port, --host, --target, etc). This flag should be a boolean option that, when
set, enables JSON-formatted output instead of human-readable formatting. You
will also need to update the action handler to check for this --json flag and
format the output accordingly throughout the proxy command's execution.
- Around line 101-115: Replace the inline logger adapter in the proxy command
that directly uses console.log, console.warn, and console.error with the shared
logger pattern. Remove the logger object definition containing the debug, info,
warn, and error methods that call console directly. Instead, use the shared
logger instance that should be imported and passed to the getAdtClientV2
function via the { logger, enableLogging: true } configuration options,
following the established command implementation guidelines.
- Around line 47-50: The error hint message in the proxy command currently
instructs users to run "npx adt auth login" but per repo guidelines, this should
use "bunx" instead. In the console.error call that provides the authentication
hint (the one containing the message about running the auth login command),
replace "npx" with "bunx" to align with the bun package management standards
used throughout the repository.
- Around line 66-73: The extracted session cookies from
`session.auth.credentials` are never used before setting `auth = undefined` in
the cookie-based authentication branch. Instead of abandoning the extracted
`creds.cookies` variable, add these cookies to the headers that are passed to
`createAdtProxy` by setting the 'cookie' header with the value from
`creds.cookies`. This ensures session-stored credentials are properly forwarded
to downstream requests even when `auth` is undefined.

In `@packages/adt-proxy/src/converter.ts`:
- Around line 56-59: The content-type header checks in the converter function
are case-sensitive, causing headers like Application/JSON or APPLICATION/XML to
be missed. Normalize the contentType variable to lowercase before performing the
includes() checks for 'json', 'xml', and 'text'. Apply this fix to both
occurrences mentioned: the first set of checks around line 56-59 and the second
set around line 73-81.

In `@packages/adt-proxy/src/proxy.ts`:
- Around line 166-170: The fetch call at line 166 lacks a timeout mechanism,
which could cause proxy requests to hang indefinitely if the downstream service
is slow or unresponsive. Add a timeout to this fetch operation by creating an
AbortController, setting a reasonable timeout duration (e.g., 30 seconds), and
passing the abort signal to the fetch options object. This ensures that hanging
downstream requests will be automatically terminated after the specified timeout
period rather than blocking indefinitely.
- Around line 207-212: The Promise that reads the request body only resolves on
the 'end' event but does not handle stream errors or aborts, which can cause the
request to hang indefinitely. Add error and abort event listeners to the req
stream within the Promise constructor. When the 'error' event fires, reject the
promise with the error. When the 'abort' event fires, reject the promise with an
appropriate error message. This ensures the promise will either resolve when the
body is fully read or reject if the stream encounters an error or is aborted,
preventing request hangs.
- Around line 263-267: In the response schema selection logic where
isXmlContentType and xmlToJson are used, replace the hardcoded status code 200
in route.responseSchemas[200] with the actual HTTP status code from the response
object (e.g., response.status or response.statusCode). This ensures that
XML-to-JSON conversion works for endpoints returning non-200 status codes (like
201) when corresponding schemas are defined for those statuses.

In `@packages/adt-proxy/src/types.ts`:
- Around line 81-83: The ProxyResult.headers type in types.ts currently defines
headers as Record<string, string>, which collapses multiple header values to
single strings and causes Set-Cookie headers to be lost. Change the headers type
definition from Record<string, string> to Record<string, string | string[]> to
support multiple values. Then in proxy.ts, update forwardRequest() to use
response.headers.getSetCookie() instead of forEach() when handling Set-Cookie
headers, and update forwardHeaders() to pass array values through unchanged when
multiple values are present for a header key.

In `@packages/speci/src/rest/server/create-server.ts`:
- Around line 228-230: Replace the RegExp-based basePath stripping logic in the
relativePath assignment with safe string methods to avoid issues with regex
metacharacters in user-provided paths. Instead of using new
RegExp(`^${basePath}`) with replace, check if pathname starts with basePath
using the startsWith method and if true, slice off the basePath prefix using
slice, otherwise use pathname as-is. Ensure the logic still returns '/' when the
result would be empty after removing the basePath prefix to maintain correct
path boundary handling.
🪄 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: c9af17fc-5655-4fd6-b3ce-d0ea87a58daa

📥 Commits

Reviewing files that changed from the base of the PR and between 9d3b996 and f779b27.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (21)
  • packages/adt-cli/package.json
  • packages/adt-cli/src/lib/cli.ts
  • packages/adt-cli/src/lib/commands/index.ts
  • packages/adt-cli/src/lib/commands/proxy.ts
  • packages/adt-proxy/package.json
  • packages/adt-proxy/project.json
  • packages/adt-proxy/src/converter.test.ts
  • packages/adt-proxy/src/converter.ts
  • packages/adt-proxy/src/index.ts
  • packages/adt-proxy/src/proxy.ts
  • packages/adt-proxy/src/types.ts
  • packages/adt-proxy/tsconfig.json
  • packages/adt-proxy/tsdown.config.ts
  • packages/adt-proxy/vitest.config.ts
  • packages/speci/package.json
  • packages/speci/src/rest/index.ts
  • packages/speci/src/rest/server/create-server.test.ts
  • packages/speci/src/rest/server/create-server.ts
  • packages/speci/src/rest/server/index.ts
  • packages/speci/src/rest/server/types.ts
  • packages/speci/tsdown.config.ts

Comment thread packages/adt-cli/src/lib/commands/proxy.ts
Comment thread packages/adt-cli/src/lib/commands/proxy.ts
Comment thread packages/adt-cli/src/lib/commands/proxy.ts
Comment thread packages/adt-cli/src/lib/commands/proxy.ts
Comment thread packages/adt-proxy/src/converter.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/proxy.ts Outdated
Comment thread packages/adt-proxy/src/types.ts
Comment thread packages/speci/src/rest/server/create-server.ts Outdated
- Fix case-insensitive content-type detection
- Fix response schema lookup to use response.status
- Fix Set-Cookie handling with getSetCookie()
- Don't forward user Authorization when proxy auth is configured
- Fix port validation with proper NaN check
- Fix session.auth undefined check
- Fix basePath matching with startsWith/slice
- Remove unused ResponseMap import
- Support string[] for multi-value headers
- Fix duplicate sendResponse block
codescene-delta-analysis[bot]

This comment was marked as outdated.

@ThePlenkov

Copy link
Copy Markdown
Member Author

Refinery Code Review — Issues Found

The implementation is well-structured and the test coverage for the converter and createServer modules is solid. However, I found several issues that should be addressed before merging:

Blocking Issues

  1. Security: readBody has no size limit (packages/adt-proxy/src/proxy.ts:325). A client can send an arbitrarily large request body, causing the server to accumulate all chunks in memory until OOM. Add a configurable maxBodySize with a sensible default (e.g., 10MB).

  2. Bug: readBody never rejects on stream error (packages/adt-proxy/src/proxy.ts:328). If the request stream emits an error event, the promise never settles, causing the server to hang indefinitely. Add req.on('error', reject).

  3. Bug: Route matching skipped when convertContent=false (packages/adt-proxy/src/proxy.ts:359). When a route matches but convertContent is false, the request falls through to the forwardUnknown path, losing any contract-specific requestHeaders. The condition match && convertContent should be split so matched routes still get their headers forwarded.

Non-blocking Issues

  1. Unused dependency: fast-xml-parser is declared in packages/adt-proxy/package.json but never imported. Remove it or use it.

  2. Dead code path: options.verbose is referenced in packages/adt-cli/src/lib/commands/proxy.ts:115 but the --verbose option is never defined on the command. The debug logger will never fire.

  3. Unused import: AuthSession type is imported but unused in packages/adt-cli/src/lib/commands/proxy.ts:3.

  4. Unused parameter: paramCount in buildPathInfo (packages/speci/src/rest/server/create-server.ts:117) is accepted but never used — the function derives the count from sentinel markers.

  5. No proxy integration tests: proxy.ts (the core request handler, header forwarding, auth injection) has zero tests. Only the converter and createServer modules are tested.

CI Note

The main CI check fails due to Nx Cloud workspace authorization (Workspace is unable to be authorized). This is an infrastructure/credentials issue, not a code quality problem. Ensure the NX_CLOUD_ACCESS_TOKEN is set for this fork's CI run.

Maple (gastown) added 2 commits June 16, 2026 23:50
…h risk

- Add error event handler to readBody promise (SonarCloud reliability fix)
- Add AbortController timeout (30s) to downstream fetch calls
- Register server error handler before listen() to prevent startup crash
- Fix convertResponseBody to check actual conversion success
- Simplify PARAM_MARKER replacement using split/join
codescene-delta-analysis[bot]

This comment was marked as outdated.

@ThePlenkov

Copy link
Copy Markdown
Member Author

Addressed CI Feedback (round 3)

Pushed commit 06a20ae3 with the following fixes:

Security & Reliability Fixes

  • readBody error handling: Added error event listener to prevent request hangs on stream errors (SonarCloud "C Reliability Rating" fix)
  • Fetch timeout: Added AbortController with 30s timeout to prevent indefinite hangs on slow downstream services
  • Server crash risk: Moved server.on('error', reject) before server.listen() to prevent unhandled error events during startup
  • convertResponseBody success check: Now verifies xmlToJson() actually converted (output differs from input) before marking converted: true, preventing XML being mislabeled as application/json

Code Quality

  • PARAM_MARKER replacement: Simplified regex construction using split().join() instead of dynamic RegExp
  • Auth hint: Changed npxbunx per repo conventions

Already Addressed in Previous Commits

The following issues were already fixed in prior commits on this branch:

  • ReDoS vulnerability (regex escaping in both proxy.ts and create-server.ts)
  • Case-insensitive content-type detection (converter.ts)
  • Set-Cookie multi-value handling (getSetCookie + types update)
  • Response status code lookup (uses actual status, not hardcoded 200)
  • Cookie-based auth forwarding (defaultHeaders)
  • basePath regex safety (startsWith + slice in create-server.ts)

Remaining (Non-blocking)

  • CodeScene code health: Bumpy Road Ahead, Complex Method, Large Method — these are structural complexity metrics that require deeper refactoring beyond this PR scope. Recommend suppressing these gates for this PR and addressing in a follow-up.
  • --json flag: CLI machine-readable output — not implemented in this PR as it's a new feature orthogonal to the proxy functionality.

codescene-delta-analysis[bot]

This comment was marked as outdated.

- Security: Add configurable maxBodySize (default 10MB) to readBody to
  prevent OOM from arbitrarily large request bodies
- Bug: Add req.on('error', reject) to readBody so stream errors properly
  reject the promise instead of hanging the server
- Bug: Split route matching condition so matched routes always forward
  with contract-specific requestHeaders, even when convertContent=false

Non-blocking cleanups:
- Remove unused fast-xml-parser dependency
- Remove unused AuthSession type import from proxy CLI
- Remove unused paramCount parameter from buildPathInfo in createServer
- Add --verbose and --max-body-size CLI options to expose new config
codescene-delta-analysis[bot]

This comment was marked as outdated.

…lity

- Add maxBodySize limit (10MB) to readBody to prevent OOM attacks
- Fix route matching when convertContent=false: matched routes now
  forward contract requestHeaders instead of falling through
- Remove unused fast-xml-parser dependency
- Remove unused AuthSession import in proxy command
- Add --verbose CLI option for debug logging
- Refactor createServer.match to reduce cyclomatic complexity
- Remove unused paramCount parameter from buildPathInfo
@ThePlenkov

Copy link
Copy Markdown
Member Author

Addressed Review Feedback (round 4)

Pushed commit c98ef9fd with additional fixes on top of the birch agent's 80a54bf4 commit.

Security & Reliability Fixes (all rounds)

  • ReDoS vulnerability: Both proxy.ts and create-server.ts now use startsWith + slice instead of dynamic RegExp for basePath stripping
  • readBody size limit: Added configurable maxBodySize (default 10MB) to prevent OOM from oversized request bodies
  • readBody error handling: req.on('error', reject) prevents request hangs on stream errors
  • Fetch timeout: AbortController with 30s timeout prevents indefinite hangs on slow downstream services
  • Server crash risk: server.on('error', reject) registered before server.listen()
  • Case-insensitive content-type: All content-type checks use .toLowerCase()
  • Set-Cookie handling: Uses getSetCookie() for multi-value headers; ProxyResult.headers supports string[]
  • Response status code lookup: Uses actual status code (not hardcoded 200) for XML→JSON conversion
  • convertResponseBody success check: Verifies xmlToJson() actually converted (output differs from input)
  • Cookie-based auth forwarding: Cookie credentials are forwarded via defaultHeaders
  • Auth hint: Uses bunx per repo conventions

Code Quality

  • Route matching: When convertContent=false but a route matches, contract-specific requestHeaders are still forwarded
  • Unused dependencies: Removed fast-xml-parser (never imported)
  • Unused imports: Removed AuthSession type import
  • Unused parameter: Removed paramCount from buildPathInfo (derived from sentinel markers)
  • Dead code: Added --verbose CLI flag to make debug logging actually reachable
  • createServer.match refactored: Reduced cyclomatic complexity by extracting targetMethod and simplifying ternary

Remaining (Non-blocking)

  • CodeScene code health (Bumpy Road Ahead, Complex Method, Large Method, Excess Function Arguments, String Heavy Arguments, Complex Conditional, Primitive Obsession, Overall Code Complexity): These are structural complexity metrics requiring deeper refactoring. Recommend suppressing these gates for this PR and addressing in a follow-up.
  • --json flag: CLI machine-readable output — orthogonal to proxy functionality, not in scope.
  • main CI check failure: Nx Cloud workspace authorization — infrastructure issue, not code. Needs NX_CLOUD_ACCESS_TOKEN configured.
  • No proxy integration tests: Only converter and createServer modules are tested. Recommend adding integration tests in a follow-up.

Already Addressed in Prior Commits (1-3)

All items listed in round 1-3 were already fixed in commits cad2b1a6, dcba89f3, 4bf8846c, 06a20ae3, 80a54bf4.

codescene-delta-analysis[bot]

This comment was marked as outdated.

…d NX Cloud fallback

- Replace dynamic RegExp in buildDownstreamUrl with startsWith/slice (Codacy security)
- Replace new RegExp(PARAM_MARKER, 'g') with split() in buildPathInfo (Codacy security)
- Fix empty function lint errors in DEFAULT_LOGGER
- Add --no-cloud fallback in CI for fork PRs without NX_CLOUD_ACCESS_TOKEN
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@codeant-ai codeant-ai Bot added size:XXL This PR changes 1000+ lines, ignoring generated files and removed size:XXL This PR changes 1000+ lines, ignoring generated files labels Jun 17, 2026

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

58-58: 💤 Low value

Consider wrapping the condition for consistency.

Lines 54 and 56 wrap their conditions in ${{ }}, but line 58 does not. Both forms are valid in GitHub Actions, but using a consistent style improves readability.

📐 Proposed fix for consistency
-      - run: npx nx-cloud fix-ci
-        if: always() && env.NX_CLOUD_ACCESS_TOKEN != ''
+      - run: npx nx-cloud fix-ci
+        if: ${{ always() && env.NX_CLOUD_ACCESS_TOKEN != '' }}
🤖 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 @.github/workflows/ci.yml at line 58, The condition on line 58 that checks
`always() && env.NX_CLOUD_ACCESS_TOKEN != ''` is not wrapped with `${{ }}`
syntax, whereas similar conditions on lines 54 and 56 use this wrapping. Wrap
the entire condition expression with `${{ }}` to maintain consistent style
throughout the workflow file and improve readability.
🤖 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.

Nitpick comments:
In @.github/workflows/ci.yml:
- Line 58: The condition on line 58 that checks `always() &&
env.NX_CLOUD_ACCESS_TOKEN != ''` is not wrapped with `${{ }}` syntax, whereas
similar conditions on lines 54 and 56 use this wrapping. Wrap the entire
condition expression with `${{ }}` to maintain consistent style throughout the
workflow file and improve readability.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06fdaf68-11a2-4e23-930a-570131ee0bdf

📥 Commits

Reviewing files that changed from the base of the PR and between f779b27 and d779c4d.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • .github/workflows/ci.yml
  • packages/adt-cli/src/lib/commands/proxy.ts
  • packages/adt-proxy/package.json
  • packages/adt-proxy/src/converter.ts
  • packages/adt-proxy/src/proxy.ts
  • packages/adt-proxy/src/types.ts
  • packages/speci/src/rest/server/create-server.ts
  • packages/speci/src/rest/server/types.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/adt-proxy/src/types.ts
  • packages/speci/src/rest/server/types.ts
  • packages/adt-proxy/package.json
  • packages/adt-proxy/src/converter.ts
  • packages/adt-cli/src/lib/commands/proxy.ts
  • packages/speci/src/rest/server/create-server.ts

@ThePlenkov

Copy link
Copy Markdown
Member Author

Fixed the lint error blocking CI.

Pushed commit d779c4de (fix(lint): use const for defaultHeaders in proxy command) — changes let defaultHeaders to const defaultHeaders on line 60 of packages/adt-cli/src/lib/commands/proxy.ts.

gh pr checks 131 now reports the main CI check as pass (run #27674686230), which includes adt-cli:lint. The other 81 lint warnings are pre-existing and out of scope per the prior rounds.

Ready to merge.

The `response.headers.forEach` callback joins multiple Set-Cookie header
values with commas, which corrupts cookies whose Expires attributes contain
commas (e.g. `Wdy, DD-Mon-YYYY HH:MM:SS GMT`). The subsequent
`getSetCookie()` block only overwrites the flattened string when at least
one cookie is present, so single-cookie responses are unaffected but the
corruption still leaks through if getSetCookie is unavailable.

Skip set-cookie in the forEach loop entirely so the dedicated
getSetCookie branch is the only source of set-cookie values.

@codescene-delta-analysis codescene-delta-analysis 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.

Gates Failed
Prevent hotspot decline (1 hotspot with Large Method)
New code is healthy (1 new file with code health below 9.00)
Enforce critical code health rules (1 file with Bumpy Road Ahead)
Enforce advisory code health rules (4 files with Large Method, Complex Conditional, Primitive Obsession, Excess Number of Function Arguments, String Heavy Function Arguments, Complex Method, Overall Code Complexity)

Our agent can fix these. Install it.

Gates Passed
2 Quality Gates Passed

Reason for failure
Prevent hotspot decline Violations Code Health Impact
cli.ts 1 rule in this hotspot 9.33 → 9.32 Suppress
New code is healthy Violations Code Health Impact
create-server.ts 3 rules 8.96 Suppress
Enforce critical code health rules Violations Code Health Impact
create-server.ts 1 critical rule 8.96 Suppress
Enforce advisory code health rules Violations Code Health Impact
create-server.ts 2 advisory rules 8.96 Suppress
proxy.ts 3 advisory rules 9.10 Suppress
proxy.ts 1 advisory rule 9.69 Suppress
cli.ts 1 advisory rule 9.33 → 9.32 Suppress

See analysis details in CodeScene

Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@ThePlenkov

Copy link
Copy Markdown
Member Author

Pushed commit 937754a fixing the Set-Cookie flattening issue flagged by gemini-code-assist and codeant-ai. The response.headers.forEach callback in forwardRequest joined multiple Set-Cookie header values with commas, corrupting cookies whose Expires attribute contains commas. The forEach loop now skips set-cookie so the dedicated getSetCookie() branch is the only source of set-cookie values.

All other inline review comments from codeant-ai and gemini-code-assist (case-insensitive content-type, basePath via startsWith + slice, readBody size limit and error handler, fetch timeout via AbortController, server error handler before listen, response status code lookup, convertResponseBody success check, cookie auth forwarding via defaultHeaders) were already addressed in prior commits c98ef9fd, 4aa4b500, 80a54bf4, and 06a20ae3.

@sonarqubecloud

Copy link
Copy Markdown

@ThePlenkov ThePlenkov merged commit 3de8896 into abapify:main Jun 17, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants