feat: add ADT proxy server with JSON↔XML conversion#131
Conversation
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
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
✅ Deploy Preview for adt-cli canceled.
|
|
Warning Review limit reached
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 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Changesspeci REST Server Extension
adt-proxy Package and CLI Integration
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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
- ReDoS Vulnerability (2 instances):
basePathparameter creates regex injection vulnerability enabling denial of service attacks - Response Conversion Bug: Only HTTP 200 responses are converted; non-200 responses with XML bodies are not converted to JSON
- Server Crash Risk: Missing early error handler registration during server startup can cause unhandled rejections
- 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
basePathvalues 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.
There was a problem hiding this comment.
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.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 2 critical |
🟢 Metrics 208 complexity · 6 duplication
Metric Results Complexity 208 Duplication 6
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.
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
- 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
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
packages/adt-cli/package.jsonpackages/adt-cli/src/lib/cli.tspackages/adt-cli/src/lib/commands/index.tspackages/adt-cli/src/lib/commands/proxy.tspackages/adt-proxy/package.jsonpackages/adt-proxy/project.jsonpackages/adt-proxy/src/converter.test.tspackages/adt-proxy/src/converter.tspackages/adt-proxy/src/index.tspackages/adt-proxy/src/proxy.tspackages/adt-proxy/src/types.tspackages/adt-proxy/tsconfig.jsonpackages/adt-proxy/tsdown.config.tspackages/adt-proxy/vitest.config.tspackages/speci/package.jsonpackages/speci/src/rest/index.tspackages/speci/src/rest/server/create-server.test.tspackages/speci/src/rest/server/create-server.tspackages/speci/src/rest/server/index.tspackages/speci/src/rest/server/types.tspackages/speci/tsdown.config.ts
- 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
Refinery Code Review — Issues FoundThe 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
Non-blocking Issues
CI NoteThe |
…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
Addressed CI Feedback (round 3)Pushed commit Security & Reliability Fixes
Code Quality
Already Addressed in Previous CommitsThe following issues were already fixed in prior commits on this branch:
Remaining (Non-blocking)
|
- 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
…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
Addressed Review Feedback (round 4)Pushed commit Security & Reliability Fixes (all rounds)
Code Quality
Remaining (Non-blocking)
Already Addressed in Prior Commits (1-3)All items listed in round 1-3 were already fixed in commits |
…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
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
58-58: 💤 Low valueConsider 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
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/ci.ymlpackages/adt-cli/src/lib/commands/proxy.tspackages/adt-proxy/package.jsonpackages/adt-proxy/src/converter.tspackages/adt-proxy/src/proxy.tspackages/adt-proxy/src/types.tspackages/speci/src/rest/server/create-server.tspackages/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
|
Fixed the lint error blocking CI. Pushed commit
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.
There was a problem hiding this comment.
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 |
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.
|
Pushed commit 937754a fixing the Set-Cookie flattening issue flagged by gemini-code-assist and codeant-ai. The All other inline review comments from codeant-ai and gemini-code-assist (case-insensitive content-type, basePath via |
|



User description
Summary
This PR adds an ADT proxy server with automatic JSON↔XML conversion to the abapify/adt-cli monorepo.
What's included:
@abapify/speci-createServer()function (packages/speci/src/rest/server/)createClient()function@abapify/adt-proxy- New package (packages/adt-proxy/)createServerbuild()methodsparse()methodsadt proxyCLI command (packages/adt-cli/)--port,--host,--target,--base-path,--no-convertoptionsArchitecture
The proxy leverages the existing contract definitions and schema
parse()/build()methods to automatically convert between JSON and XML. This means:Usage
Tests
speci/rest/server(route extraction and matching)adt-proxyconverter moduleNext Steps
This PR sets the foundation for:
Summary by cubic
Adds
@abapify/adt-proxy, a schema‑aware ADT proxy that converts JSON↔XML, plus anadt proxyCLI andcreateServer()in@abapify/specifor server‑side route extraction. Improves security, routing, and header handling with body-size limits, timeouts, safe base paths, and correctSet-Cookieforwarding.New Features
@abapify/adt-proxy: Converts JSON bodies to XML and XML responses to JSON via schemabuild/parse; forwards unmatched routes; configurable target/auth/base path/conversion/forwarding/maxBodySize(default 10MB); programmatic API; exportsjsonToXml/xmlToJson.adt proxyCLI: Start the proxy with--port,--host,--target,--base-path,--no-convert,--no-forward-unknown,--max-body-size,--verbose; uses current auth session when--targetis 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
maxBodySize(10MB default) with stream error handling; 30s downstream fetch timeout; register servererrorbeforelisten(); robust port validation and auth/session guards; CI fallback with--no-cloudwhenNX_CLOUD_ACCESS_TOKENis missing.requestHeaderseven when conversion is disabled; safe base‑path checks withstartsWith/slice; simpler path‑param handling viasplit/join; correct multi‑value headers, includingSet-CookieviagetSetCookie()and skippingset-cookiein header iteration to avoid cookie corruption; do not forward clientAuthorizationwhen proxy auth is set.response.status; only mark responses as converted when body changes; add--verboselogging; remove unusedfast-xml-parser; reduce matcher complexity increateServer().Written for commit 937754a. Summary will update on new commits.
Summary by CodeRabbit
proxyCLI command to start an ADT proxy server with JSON↔XML conversion.CodeAnt-AI Description
Add an ADT proxy command that forwards requests to SAP with automatic JSON↔XML conversion
What Changed
adt proxycommand to start a local ADT proxy server, with options for target URL, host, port, base path, body size limit, and turning conversion offspeciso server code can match incoming requests against ADT contract pathsImpact
✅ 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:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
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:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
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.