fix: require object JSON for machine passport writes

[Asti1982]

Summary

• require JSON object bodies on the Machine Passport write/compute endpoints that need fields or fingerprint data
• keep the existing optional-object behavior for attestation/benchmark event routes
• add regressions for non-object payloads on create, update, repair-log, lineage, and compute-machine-id
• keep the wallet network-regression test offline by mocking the CLI's imported urlopen symbol

Root cause

Some Machine Passport routes called request.get_json() and then either used .get() or passed the payload into compute_machine_id() without checking that the JSON value was an object. A non-empty JSON array could therefore either produce a 500 (/api/machine-passport with ['name', 'owner_miner_id']) or be accepted as fingerprint input (/compute-machine-id) even though the API documents object payloads.

The first CI attempt also exposed an unrelated flaky wallet regression test: it patched urllib.request.urlopen, but rustchain_cli imports urlopen directly, so the test could hit the live network and time out in CI. The test now patches rustchain_cli.urlopen and asserts the controlled CLI exit path.

Verification

• python -m pytest tests\\test_wallet_show_regression.py::TestWalletBalanceEndpoint::test_wallet_show_handles_network_error_gracefully tests\\test_machine_passport_event_json_validation.py node\\tests\\test_machine_passport.py -q -> 37 passed
• python -m pytest tests\\test_machine_passport_event_json_validation.py -q -> 10 passed
• python -m pytest node\\tests\\test_machine_passport.py tests\\test_machine_passport_event_json_validation.py -q -> 36 passed
• python -m py_compile node\\machine_passport_api.py tests\\test_machine_passport_event_json_validation.py tests\\test_wallet_show_regression.py -> passed
• python tools\\bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK
• git diff --check -> passed

Note: full local pytest -q on Windows is not clean in this checkout because many existing temp-file teardown tests hold SQLite/temp files open and optional integrations are missing. The targeted paths above cover this PR and the CI failure reproduced from GitHub.

[github-actions]

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

• Your PR has a BCOS-L1 or BCOS-L2 label
• New code files include an SPDX license header
• You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

[saim256]

Review for Scottcjn/rustchain-bounties#73.

I reviewed the Machine Passport JSON object validation change. The patch fixes the unsafe boundary cleanly: required write/compute routes now reject non-object JSON before using .get() or passing payloads into compute_machine_id(), while the attestation/benchmark event routes keep their existing optional-object behavior.

Validation I ran:

• python -m pytest tests\test_machine_passport_event_json_validation.py -q -> 10 passed
• python -m pytest node\tests\test_machine_passport.py tests\test_machine_passport_event_json_validation.py -q -> 36 passed
• python -m py_compile node\machine_passport_api.py tests\test_machine_passport_event_json_validation.py -> passed
• python tools\bcos_spdx_check.py --base-ref origin/main -> OK
• git diff --check origin/main...HEAD -- node/machine_passport_api.py tests/test_machine_passport_event_json_validation.py -> passed
• python -m ruff check node\machine_passport_api.py tests\test_machine_passport_event_json_validation.py --select E9,F821,F811 --output-format=concise -> passed

The new parametrized regression covers all changed required-object routes, and the existing event-route tests still confirm list/no-body behavior for the optional payload paths. I do not see a merge blocker.

[godd-ctrl]

Approved current head 7f949bd.

The machine passport write/compute endpoints now reject non-object JSON bodies before field access or machine-id computation, while the attestation and benchmark event routes keep their optional-empty-object behavior. The shared helper keeps the route behavior consistent, and the added regressions cover create, update, repair-log, lineage, and compute-machine-id non-object payloads.

Validation performed:

• git diff --name-status origin/main...HEAD -> node/machine_passport_api.py, tests/test_machine_passport_event_json_validation.py
• git diff --check origin/main...HEAD -> passed
• python tools\bcos_spdx_check.py --base-ref origin/main -> passed
• python -m py_compile node\machine_passport_api.py tests\test_machine_passport_event_json_validation.py -> passed
• python -m pytest tests\test_machine_passport_event_json_validation.py -q -> 10 passed
• python -m pytest node\tests\test_machine_passport.py tests\test_machine_passport_event_json_validation.py -q -> 36 passed

No blocking issues found in the reviewed diff.

[saim256]

Follow-up review for Scottcjn/rustchain-bounties#73 on current head 3cd2a63225507334425d4a84d725e2bd3e4875bb.

The new wallet regression commit looks correct. tools/cli/rustchain_cli.py imports urlopen directly (from urllib.request import urlopen, Request), so patching urllib.request.urlopen did not isolate fetch_api() and could still reach the live node. The updated test patches rustchain_cli.urlopen, which is the symbol actually used by fetch_api(), and now asserts the controlled SystemExit(1) path instead of allowing an uncontrolled network exception.

Validation I ran on the updated head:

• python -m pytest tests\test_wallet_show_regression.py::TestWalletBalanceEndpoint::test_wallet_show_handles_network_error_gracefully tests\test_machine_passport_event_json_validation.py node\tests\test_machine_passport.py -q -> 37 passed
• python -m py_compile node\machine_passport_api.py tests\test_machine_passport_event_json_validation.py tests\test_wallet_show_regression.py -> passed
• git diff --check origin/main...HEAD -- node/machine_passport_api.py tests/test_machine_passport_event_json_validation.py tests/test_wallet_show_regression.py -> passed
• python tools\bcos_spdx_check.py --base-ref origin/main -> OK
• Source check confirmed fetch_api() uses the imported urlopen symbol from tools/cli/rustchain_cli.py.

I do not see a blocker in the updated PR.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[508704820]

Code Review: Require Object JSON for Machine Passport Writes

Summary

Adds get_required_json_object() helper that validates both JSON presence and type (must be dict/object). Replaces inline validation in create_passport().

What Works Well

1. Type validation: Rejects JSON arrays, strings, numbers — only objects accepted
2. Consistent with get_optional_json_object(): Mirrors existing helper pattern
3. DRY: Replaces inline validation code with reusable function
4. Clear error messages: "JSON object required" vs "JSON body required"

Issues Found

1. Low — get_required_json_object should be used for ALL write endpoints
Only create_passport() is updated. Other write endpoints (update_passport, revoke_passport) may still use get_json() without type checking. Consider applying consistently.

Verdict: Approve

Good input validation improvement. Apply to all write endpoints for consistency.

[jaxint]

Great contribution! LGTM.

[508704820]

Code Review: Require object JSON for machine passport writes

Summary

Good refactor — extracts repeated JSON body validation into get_required_json_object() helper and adds type checking (isinstance(data, dict)) to prevent arrays/primitives from being accepted.

Positive ✅

1. DRY improvement: Eliminates duplicated JSON validation code in create_passport and update_passport
2. Type safety: isinstance(data, dict) prevents accepting JSON arrays which could cause downstream errors
3. Consistent error format: Uses same {ok, error, message} format as existing endpoints
4. Consistent with existing pattern: Mirrors get_optional_json_object() design

Issues 🔍

1. (LOW) get_required_json_object() doesn't check for empty dict {} — an empty object passes validation but might not have required fields. However, the required field check happens right after, so this is fine.

2. (INFO) Could also validate Content-Type header is application/json for extra safety, but get_json(silent=True) handles this gracefully.

LGTM ✅ Good refactor, improves code quality and type safety.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[saim256]

Approved after focused validation. The new required-object helper keeps the previous missing-body handling while rejecting array JSON before route-specific field checks, and the parametrized regression covers create/update/repair/lineage/compute-machine-id routes. I also verified the adjusted wallet regression patches the imported
ustchain_cli.urlopen symbol, so it is now testing the actual call site.\n\nChecks run locally on the PR branch:\n- python -m pytest tests/test_machine_passport_event_json_validation.py tests/test_wallet_show_regression.py -q -> 15 passed\n- python -m py_compile node/machine_passport_api.py tests/test_machine_passport_event_json_validation.py tests/test_wallet_show_regression.py -> passed\n- git diff --check origin/main...HEAD -- node/machine_passport_api.py tests/test_machine_passport_event_json_validation.py tests/test_wallet_show_regression.py -> passed\n- python tools\bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK

[loganoe]

Approved. The shared required-object helper closes the non-object JSON paths on the machine passport write/compute routes while leaving the optional event routes covered by the existing optional-object behavior. The wallet regression update also correctly patches the imported urlopen symbol so the test stays offline.

Validation performed:

• git diff --check origin/main...HEAD
• python3 -m py_compile node/machine_passport_api.py tests/test_machine_passport_event_json_validation.py tests/test_wallet_show_regression.py
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /tmp/rustchain-flask-venv/bin/python -m pytest -q tests/test_machine_passport_event_json_validation.py -> 10 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /tmp/rustchain-flask-venv/bin/python -m pytest -q tests/test_wallet_show_regression.py -> 5 passed