Skip to content

fix(client): security hardening - #479

Merged
rodrigojara-cko merged 2 commits into
masterfrom
fix/INT-1769-security-hardening
Nov 6, 2026
Merged

rodrigojara-cko merged 2 commits into
masterfrom
fix/INT-1769-security-hardening

Conversation

@rodrigojara-cko

@rodrigojara-cko rodrigojara-cko commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Security hardening: input validation and encoding when building request URLs. Path values are percent-encoded per segment and query strings are built with the shared buildQueryParams helper. Reported internally.

Changes

  • src/services/utils.js: new encodePathSegment helper, next to buildQueryParams.
  • src/api/balances/balances.js, src/api/disputes/disputes.js, src/api/events/events.js, src/api/financial/financial.js, src/api/payment-methods/payment-methods.js, src/api/payments/payments.js, src/api/platforms/payment-instruments.js, src/api/reconciliation/reconciliation.js, src/api/reports/reports.js: path values encoded with encodePathSegment; query strings built with buildQueryParams; @throws added to the JSDoc.
  • src/api/apm-specific/baloto.js, src/api/apm-specific/boleto.js, src/api/apm-specific/fawry.js, src/api/apm-specific/klarna.js, src/api/apm-specific/oxxo.js, src/api/apm-specific/pagofacil.js, src/api/apm-specific/rapipago.js, src/api/apm-specific/sepa.js, src/api/compliance-requests/compliance-requests.js, src/api/customers/customers.js, src/api/files/files.js, src/api/forward/forward.js, src/api/google-pay/google-pay.js, src/api/hosted-payments/hosted-payments.js, src/api/identities/address-document-verifications.js, src/api/identities/aml-screenings.js, src/api/identities/applicants.js, src/api/identities/face-authentications.js, src/api/identities/id-document-verifications.js, src/api/identities/identity-verifications.js, src/api/instruments/instruments.js, src/api/inventory/inventory.js, src/api/issuing/cardholders.js, src/api/issuing/cards.js, src/api/issuing/control-groups.js, src/api/issuing/control-profiles.js, src/api/issuing/controls.js, src/api/issuing/digital-cards.js, src/api/issuing/disputes.js, src/api/issuing/simulate.js, src/api/issuing/transactions.js, src/api/network-tokens/network-tokens.js, src/api/onboarding-simulator/onboarding-simulator.js, src/api/payment-contexts/payment-contexts.js, src/api/payment-sessions/payment-sessions.js, src/api/payment-setups/payment-setups.js, src/api/payments-links/payments-links.js, src/api/platforms/entity-requirements.js, src/api/platforms/files.js, src/api/platforms/payout-schedules.js, src/api/platforms/reserve-rules.js, src/api/platforms/subentity.js, src/api/sessions/sessions.js, src/api/tokens/tokens.js, src/api/transfers/transfers.js, src/api/webhooks/webhooks.js, src/api/workflows/workflows.js: path values encoded with encodePathSegment; @throws added to the JSDoc.
  • test/services/utils.js: tests for encodePathSegment.
  • test/services/path-segments-unit.js: one parametrised suite over every method that puts a caller value in the request path (exact path for a valid id, encoded values, rejected values).
  • test/balances/balances-balances-at-unit.js, test/balances/balances-unit.js, test/disputes/disputes-unit.js, test/events/events.js, test/financial/financial.js, test/payment-methods/payment-methods-unit.js, test/payments/getPaymentList.js, test/platforms/payment-instruments/payment-instruments-unit.js, test/reconciliation/reconciliation-unit.js, test/reports/reports-unit.js: query encoding tests for each method that builds a query string; the balances assertion now expects the encoded colon.

API Reference

None. No request or response schema changes.

Breaking changes

Path and query values are now percent-encoded by the SDK. Callers that already encoded a value themselves (for example with encodeURIComponent) should pass the raw value instead, otherwise it is encoded twice and the API will not find it. Values without special characters, such as pay_, ent_ or cus_ ids, go out unchanged. Undefined, null, empty, . and .. path values now throw ValueError before any request is sent (the API answered 404 for them).

Released as a minor; reviewers, please flag it if you think this needs a major.

Tests

  • Existing tests pass (unit and integration)
  • Serialization tests cover every new or changed property: no models changed; new tests cover the encoding helper, every method that builds a request path, and every method that builds a query string
  • README updated if documented usage changed: no documented usage changed

Related

None.

Input validation for request building. Values placed in request paths are
percent-encoded per segment, and undefined, empty, "." and ".." path values
throw ValueError before any request is sent. Query strings are built with the
shared buildQueryParams helper, so their values are percent-encoded too.

Values without special characters, such as resource ids, go out unchanged.
@rodrigojara-cko
rodrigojara-cko requested a review from a team November 5, 2026 12:41
@agent-wall-e

agent-wall-e Bot commented Nov 5, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:src/api/issuing/cardholders.js
  • security_sensitive_path:src/api/sessions/sessions.js
  • security_sensitive_path:src/api/tokens/tokens.js

Operational gates

  • ✅ jira_ticket (INT-1769)
  • ✅ independent_review

Files analysed: 69


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Nov 5, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_path — src/api/issuing/cardholders.js classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path — src/api/sessions/sessions.js classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path — src/api/tokens/tokens.js classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🔵 Advisory review: Sound, but needs your judgement

This PR needs a human approval. The code itself reads as correct; whether it should land depends on context I don't have.

This PR adds encodePathSegment and buildQueryParams usage across all SDK API files to prevent path traversal and query injection, but the implementation of encodePathSegment in src/services/utils.js is not visible in the truncated diff — a reviewer must verify the encoding logic, the ValueError class definition, and the exact rejection rules before approving.

For you to decide

  • The implementation of encodePathSegment in src/services/utils.js is not shown; a reviewer must confirm it uses encodeURIComponent (not encodeURI or a weaker function) and that the rejection set (null/undefined/empty/./..) is correct.
  • The ValueError thrown by encodePathSegment is referenced in JSDoc but its definition is not visible; confirm it is a real exported class (not Error aliased or undefined), so callers can catch it by type.
  • The test files (test/services/utils.js, test/services/path-segments-unit.js) are claimed in the PR description but not visible in the diff; a reviewer should confirm they exist, run, and cover at least: safe ids pass through unchanged, special characters are encoded, and null/empty/dot inputs throw.
  • The balances retrieve method previously called encodeURIComponent(options.balancesAt) explicitly; that call is removed and the value now goes through buildQueryParams — a reviewer must confirm buildQueryParams applies equivalent or stronger encoding so this is not a regression.
  • This is a declared breaking change (double-encoding for callers who pre-encode): the PR proposes a minor version bump; a reviewer with knowledge of the SDK's consumer base should decide if that is appropriate or if a major version is warranted.
  • The disputes.get(body) rewrite correctly replaces the unsafe key=value join with buildQueryParams, but the old code had no encoding at all — confirm buildQueryParams encodes both keys and values, not just values.
  • The diff is truncated (47 files omitted); a reviewer should scan the omitted files to confirm the encodePathSegment pattern was applied uniformly and no call sites were missed.

⚠️ The diff was too large to read in full, so this review covers only part of the change.


This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02

@agent-wall-e

agent-wall-e Bot commented Nov 5, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:src/api/issuing/cardholders.js
  • security_sensitive_path:src/api/sessions/sessions.js
  • security_sensitive_path:src/api/tokens/tokens.js

Operational gates

  • ✅ jira_ticket (INT-1769)
  • ✅ independent_review

Files analysed: 69


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Nov 5, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_path — src/api/issuing/cardholders.js classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path — src/api/sessions/sessions.js classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path — src/api/tokens/tokens.js classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

@rodrigojara-cko
rodrigojara-cko merged commit bc3c1d9 into master Nov 6, 2026
6 checks passed
@rodrigojara-cko
rodrigojara-cko deleted the fix/INT-1769-security-hardening branch November 6, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants