Reject raw APDU commands in KMP NFC bridge

[transphorm]

Summary

This PR hardens the KMP NFC bridge by rejecting caller-supplied apduCommands at the handler boundary on both Android and iOS. The KMP SDK continues to support only the
existing high-level passport scan flow, which keeps the native handler thin and avoids exposing raw APDU passthrough.

It adds shared tests covering accepted standard scan params and rejected APDU input, and updates the native-shells plan/spec to mark NS-04 complete and document the
implemented policy.

Test plan

---

Native Consolidation Checklist

• CONTRACTS.md reviewed - no unintended contract changes
• Layer 1 bridge contract tests pass (cd app && yarn jest:run / yarn workspace @selfxyz/rn-sdk-test-app test)
• Layer 3 builds pass (app iOS, RN test app iOS, RN test app Android)
• Layer 4 manual smoke test signed off (if consolidation PR)
• No new native business logic added (logic belongs in TypeScript)

Summary by CodeRabbit

• Bug Fixes

  • Strengthened NFC security by implementing parameter validation at the bridge boundary on Android and iOS. Caller-supplied APDU commands are now rejected to prevent unauthorized execution, while standard passport scan operations remain fully supported.

• Tests

  • Added comprehensive test coverage for parameter validation, including standard scan scenarios and rejection cases.

[chatgpt-codex-connector]

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

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements a security boundary policy that rejects caller-supplied APDU commands at the KMP NFC bridge level. The NfcApduPolicy utility validates parameters on both Android and iOS platforms, ensuring only the built-in passport scan sequence is allowed while preventing raw APDU passthrough.

Changes

Cohort / File(s)	Summary
NFC APDU Policy Implementation packages/kmp-sdk/shared/src/commonMain/kotlin/xyz/self/sdk/handlers/NfcApduPolicy.kt	New internal object introducing boundary security validation. The requireSupportedParams() function rejects non-empty apduCommands with BridgeHandlerException (code: NFC_APDU_NOT_ALLOWED), allowing only empty/null/absent commands.
Bridge Handler Integration packages/kmp-sdk/shared/src/androidMain/kotlin/xyz/self/sdk/handlers/NfcBridgeHandler.kt, packages/kmp-sdk/shared/src/iosMain/kotlin/xyz/self/sdk/handlers/NfcBridgeHandler.kt	Added upfront parameter validation via NfcApduPolicy.requireSupportedParams(params) at the start of the scan() function on both platforms.
Policy Test Coverage packages/kmp-sdk/shared/src/commonTest/kotlin/xyz/self/sdk/handlers/NfcApduPolicyTest.kt	New test suite validating acceptance of standard passport params and empty APDU lists, and rejection of non-empty or malformed apduCommands with correct error codes and messaging.
Documentation Updates specs/projects/sdk/workstreams/native-shells/SPEC.md, specs/projects/sdk/workstreams/native-shells/plans/NS-04-apdu-allowlist.md	Status marked Done with completed implementation notes. Updated motivation from generic allowlisting to boundary security policy, expanded validation scope, and added status log entry describing the bridge-boundary APDU rejection mechanism.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• feat: introduce kmp sdk & demo app #1749: Introduces the original NfcBridgeHandler.scan() flow on both Android and iOS platforms—these changes add the upstream parameter validation to that same handler.

Suggested reviewers

• seshanthS

Poem

🛡️ At the bridge, a guardian stands tall,
Rejecting raw APDU's call,
Only passport scans pass through,
Boundary policy—crisp and true! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description includes a Summary section explaining the changes and objectives, and mentions test coverage. However, the Tested section is empty, and the How to QA section is missing entirely, failing to meet template requirements.	Complete the Tested section with details on how tests were run (unit tests, manual testing), and add a How to QA section with repeatable testing steps.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: rejecting raw APDU commands in the KMP NFC bridge, which is the primary objective of this PR.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin/kmp-ns-04

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

packages/kmp-sdk/shared/src/commonTest/kotlin/xyz/self/sdk/handlers/NfcApduPolicyTest.kt (1)

14-95: Good test coverage with one potential gap.

The tests cover the key scenarios:

• Standard params (no apduCommands) → allowed
• Empty apduCommands array → allowed
• Non-empty apduCommands → rejected with correct code/message
• Malformed apduCommands (string) → rejected

One scenario not explicitly tested: "apduCommands": null (JsonNull). The policy handles this case (Line 19 in NfcApduPolicy.kt), but there's no test asserting it. Consider adding for completeness:

🧪 Optional: Add JsonNull test case

`@Test`
fun requireSupportedParams_allowsNullApduCommands() {
    NfcApduPolicy.requireSupportedParams(
        params(
            """
            {
              "passportNumber": "L898902C3",
              "dateOfBirth": "690806",
              "dateOfExpiry": "060815",
              "apduCommands": null
            }
            """.trimIndent(),
        ),
    )
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/kmp-sdk/shared/src/commonTest/kotlin/xyz/self/sdk/handlers/NfcApduPolicyTest.kt`
around lines 14 - 95, Add a test that verifies
NfcApduPolicy.requireSupportedParams accepts a JsonNull apduCommands value:
create a new test (e.g. requireSupportedParams_allowsNullApduCommands) that
calls NfcApduPolicy.requireSupportedParams with the same params helper
(params(rawJson)) but with "apduCommands": null in the JSON payload; this should
assert no exception is thrown (same style as the existing allowed-case tests) to
cover the JsonNull branch handled in NfcApduPolicy.requireSupportedParams.

specs/projects/sdk/workstreams/native-shells/plans/NS-04-apdu-allowlist.md (1)

1-4: Minor: Title says "Allowlist" but implementation rejects all APDU commands.

The plan title references "APDU Allowlist" but the actual implementation rejects all caller-supplied APDU commands (no allowlist exists—it's a complete deny). The content correctly describes the rejection policy, so this is just a naming inconsistency. Consider updating to "APDU Rejection" or "APDU Boundary Policy" for clarity, though this is low priority.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/projects/sdk/workstreams/native-shells/plans/NS-04-apdu-allowlist.md`
around lines 1 - 4, The document title "APDU Allowlist in KMP NFC Bridge
Handler" is misleading because the implementation rejects all APDU commands;
update the header to reflect the actual behavior (e.g., "APDU Rejection in KMP
NFC Bridge Handler" or "APDU Boundary/Rejection Policy") and adjust any short
description lines (the one-line summary after the header and the metadata block)
so the name and status consistently describe the deny-all APDU policy used by
this plan.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@packages/kmp-sdk/shared/src/commonTest/kotlin/xyz/self/sdk/handlers/NfcApduPolicyTest.kt`:
- Around line 14-95: Add a test that verifies
NfcApduPolicy.requireSupportedParams accepts a JsonNull apduCommands value:
create a new test (e.g. requireSupportedParams_allowsNullApduCommands) that
calls NfcApduPolicy.requireSupportedParams with the same params helper
(params(rawJson)) but with "apduCommands": null in the JSON payload; this should
assert no exception is thrown (same style as the existing allowed-case tests) to
cover the JsonNull branch handled in NfcApduPolicy.requireSupportedParams.

In `@specs/projects/sdk/workstreams/native-shells/plans/NS-04-apdu-allowlist.md`:
- Around line 1-4: The document title "APDU Allowlist in KMP NFC Bridge Handler"
is misleading because the implementation rejects all APDU commands; update the
header to reflect the actual behavior (e.g., "APDU Rejection in KMP NFC Bridge
Handler" or "APDU Boundary/Rejection Policy") and adjust any short description
lines (the one-line summary after the header and the metadata block) so the name
and status consistently describe the deny-all APDU policy used by this plan.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f5a5ea37-88b8-4eda-8583-f698b15f7639

📥 Commits

Reviewing files that changed from the base of the PR and between 6dcaa63 and d599f08.

📒 Files selected for processing (6)

• packages/kmp-sdk/shared/src/androidMain/kotlin/xyz/self/sdk/handlers/NfcBridgeHandler.kt
• packages/kmp-sdk/shared/src/commonMain/kotlin/xyz/self/sdk/handlers/NfcApduPolicy.kt
• packages/kmp-sdk/shared/src/commonTest/kotlin/xyz/self/sdk/handlers/NfcApduPolicyTest.kt
• packages/kmp-sdk/shared/src/iosMain/kotlin/xyz/self/sdk/handlers/NfcBridgeHandler.kt
• specs/projects/sdk/workstreams/native-shells/SPEC.md
• specs/projects/sdk/workstreams/native-shells/plans/NS-04-apdu-allowlist.md