Skip to content

Implement contract-sender warning for source C-addresses in extractRouting - #326

Merged
codeZe-us merged 1 commit into
Boxkit-Labs:mainfrom
Elizzy01:feat/contract-sender-warning
Sep 29, 2026
Merged

codeZe-us merged 1 commit into
Boxkit-Labs:mainfrom
Elizzy01:feat/contract-sender-warning

Conversation

@Elizzy01

@Elizzy01 Elizzy01 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Closes #321

Overview

Adds contract sender detection logic to extractRouting to identify contract source accounts (C...), clear the routing state, and emit a structured CONTRACT_SENDER_DETECTED warning.

Key Changes

  • Routing Engine (extract.ts):
    • Checks input.sourceAccount and identifies contract addresses (parsed.kind === "C").
    • Resets routing outputs (destinationBaseAccount: null, routingId: null, routingSource: "none").
    • Emits CONTRACT_SENDER_DETECTED warning with info severity, filtered by minSeverityLevel and preserving existing sanitization warnings.
    • Safely ignores unparseable/invalid sourceAccount entries.
  • Warning Definitions (warnings.ts):
    • Registered CONTRACT_SENDER_DETECTED message mapping.
  • Tests (extract.test.ts):
    • Added unit tests covering contract sender routing clearing, warning emission, severity filtering, and invalid address fallbacks.

Summary by CodeRabbit

  • Bug Fixes
    • Contract addresses used as the source are now detected, and routing state is cleared with a warning.
    • Invalid source-account values no longer prevent routing from proceeding based on the destination.
    • Contract destinations continue to return an error rather than a routing result.

@drips-wave

drips-wave Bot commented Sep 29, 2026

Copy link
Copy Markdown

@Elizzy01 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 830a33e4-4c4f-4927-b3aa-b5c0b2f1994a

📥 Commits

Reviewing files that changed from the base of the PR and between 6c38bd0 and 2368064.

📒 Files selected for processing (3)
  • packages/core-ts/src/routing/extract.ts
  • packages/core-ts/src/routing/warnings.ts
  • packages/core-ts/src/test/extract.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

extractRouting now checks whether sourceAccount is a contract address. If it is, the result has cleared routing fields and a severity-filtered contract-sender warning. Invalid source-account strings do not stop destination-based routing.

Changes

Contract-source detection

Layer / File(s) Summary
Detect contract sources and verify routing results
packages/core-ts/src/routing/extract.ts, packages/core-ts/src/routing/warnings.ts, packages/core-ts/src/test/extract.test.ts
extractRouting returns cleared routing fields for a contract source and includes the contract-sender warning. Tests cover warning order and filtering, contract destinations, and routing after an invalid source-account string.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: madisonsc52-del

Merge Risk: ⚪ Minimal · up to 23680

No actionable merge-blocking risk is established. The change flags contract-origin payments while preserving routing for invalid source-account strings; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 23680

The change is contained within the routing library, but contract-source inputs now suppress destination-error reporting and optional required-memo diagnostics. Cleared routing remains non-routable; whether downstream applications also treat it as a rejection is not established.

Retained concerns

  • Low · security · inferred: A caller-supplied, valid contract source now suppresses destination checksum-error reporting and the optional required-memo diagnostic. At warning-level filtering, the cleared result can contain no explanatory warning. Consumers that interpret absent diagnostics as approval could lose these signals; that downstream interpretation is not established.
Security review details

Security Blast Radius

  • inferred — The established exposure is routing results and diagnostics for callers supplying a valid contract source, including the transaction adapter and async wrapper. The inspected path does not demonstrate transaction submission, credential access, or cross-tenant authority; external consumers may add consequences not covered here.

Security Findings and Attack Paths

  • inferred — A caller controlling sourceAccount can select a valid C-address to avoid destination checksum diagnostics and the async memo-requirement lookup. Exploitation would additionally require a downstream consumer to accept cleared routing or absent warnings as approval; no such consumer was established.

Trust Boundaries and Controls

  • observed — The source branch does not bypass destination type or prefix guards and produces no usable base account or routing ID. Invalid source strings do not activate clearing. These controls limit the demonstrated effect to lost diagnostics rather than newly authorized routing.

Resilience and Maintainability Implications

  • observed — The optional memo-requirement check already preserved synchronous results on network or configuration failure. That advisory behavior predates this PR; the introduced effect is deterministic suppression of the check for contract-source results.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a contract-sender warning for source C-addresses in extractRouting.
Linked Issues check ✅ Passed Issue #321 requires extractRouting to parse input.sourceAccount and append a contract-sender warning for a C-address. The PR adds this check, clears the routing outputs, emits `CONTRACT_SENDER_DET…
Out of Scope Changes check ✅ Passed The changes are limited to the extractRouting implementation, the related warning-message registry, and automated tests for the contract-sender behavior. These changes directly support Issue #321. N…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@codeZe-us
codeZe-us self-requested a review September 29, 2026 22:51
@codeZe-us
codeZe-us merged commit 9173ed4 into Boxkit-Labs:main Sep 29, 2026
2 checks passed
islarmeeyah-cyber pushed a commit to islarmeeyah-cyber/stellar-address-kit that referenced this pull request Oct 12, 2026
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.

Implement contract-sender warning for source C-addresses in extractRouting

2 participants