Skip to content

feat(tronscan): add security checks and validation - #76

Open
zuojr wants to merge 1 commit into
BofAI:developfrom
zuojr:feature/tronscan-security-checks
Open

feat(tronscan): add security checks and validation#76
zuojr wants to merge 1 commit into
BofAI:developfrom
zuojr:feature/tronscan-security-checks

Conversation

@zuojr

@zuojr zuojr commented Aug 29, 2026

Copy link
Copy Markdown

Summary

  • Add a read-only security.js command for TronScan account, token, URL, transaction, multi-signature permission, and token-approval security signals.
  • Validate TRON Base58Check addresses, token identifiers, transaction hashes, and URLs locally before querying; strip URL query strings and fragments before transmission.
  • Return a normalized assessment while preserving the complete upstream response, and report incomplete responses as unknown rather than a clean result.
  • Require token identifiers for TRC10/TRC20 address-history queries and update their smoke tests to match the current TronScan API contract.
  • Rebuild the contribution from the latest develop as requested in feat(tronscan): add security checks and validation #71.

Branch routing

  • Ordinary work targets develop from feature/*.
  • A main PR comes only from release_* or hotfix/*.
  • Release and hotfix changes will also be merged into develop.

Skill checks

  • Changed SKILL.md files have valid YAML frontmatter.
  • Dependencies and supported versions are documented consistently.
  • Examples, resources, JSON files, and scripts were validated where applicable.
  • Security, confirmation, secret-handling, network, and fee implications were reviewed.
  • README and changelog entries were updated when behavior or the public catalog changed.

Verification

Commands run from tronscan-skill/:

node --check scripts/security.js
node --check scripts/transfer.js
node --check scripts/test_all.js
node tests/security.test.js
node scripts/test_all.js

Results:

  • Offline validation and assessment tests: 11/11 passed
  • Live smoke tests across all configured TronScan endpoints: 30/30 passed
  • Skill frontmatter, JSON resources, package metadata, changed-file scope, and git diff --check passed
  • Branch comparison against develop: one commit ahead, zero behind

Release workflow

  • Keep SKILL.md, package.json, and package-lock.json at the current 1.0.0 version.
  • Record the behavior changes under Unreleased instead of creating a dated 1.1.0 release entry.
  • PR feat!: consolidate TRON skills and remove Twitter skills #75 is still open and has not added private: true to develop yet. If it lands before this PR, that field will be preserved while synchronizing the branch.

Security

  • All new API operations are read-only GET requests.
  • No wallet, signing, private-key, or transaction-broadcast path is introduced.
  • URL credentials are rejected; query strings and fragments are removed before transmission.
  • no_known_flags is documented as not being proof that a target is safe.

Previous PR

This replaces #71 following the maintainer-requested branch and release-workflow migration.

@roger-gan roger-gan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the read-only security checks. The direction is useful, but I found several paths where the normalized result can be misleading, plus one advertised mode that is currently unusable through the default backend.

  1. TRC10 security checks fail through the documented default proxy. requireTokenIdentifier accepts numeric TRC10 IDs and the skill advertises token <token-id-or-contract>, but with no TRONSCAN_API_KEY configured, npm run security -- token 1002000 returns HTTP 400 through ts.bankofai.io. The same ID succeeds with npm run token -- --trc10 1002000 and resolves to BTTOLD, so the ID is valid. The live security smoke test only covers a TRC20 contract and misses this branch. Please fix the proxy/API integration or stop advertising TRC10 support, and add a live TRC10 security test.

  2. Documented unknown token values become no_known_flags. In assessToken, token_level: "0", black_list_type: 0, and increase_total_supply: 0 add no signals. TronScan defines these values as unknown/unrecognized. With otherwise benign fields the function returns no_known_flags, even though the security attributes are unresolved. Please return unknown with explicit reasons for these enum values and for invalid field types.

  3. Partial transaction records are treated as resolved and clean. assessTransactions({ [hash]: {} }, [hash]) currently returns no_known_flags with an empty unresolved list. Key presence marks the hash as resolved, while the required riskToken, zeroTransfer, riskAddress, sameTailAttach, and riskTransaction fields are never validated. A missing, null, or wrongly typed required field should make that hash unknown, and this case needs an offline test.

  4. The other assessors check property presence but not response validity. hasAllOwnFields accepts required fields whose values are null or the wrong type. For example, four null account flags and cheat_url: null both become no_known_flags; multiSign: null is reported as multi-signature disabled. assessApprovals also reports clean when the three counters exist but the required riskApprove array is missing. Please use per-mode schema/type validation so incomplete upstream responses consistently fail closed as unknown.

  5. One transfer example is now invalid. The example in scripts/transfer.js still runs --trc20 ... --direction 2 without --token; after this PR it exits with --trc20 requires --token <contract>. Please update that runnable example.

Verification on commit f4073a4: npm test passed 11/11, npm run test:live passed 30/30, syntax checks and git diff --check passed. The targeted cases above expose gaps not covered by those tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants