feat(tronscan): add security checks and validation - #76
Conversation
roger-gan
left a comment
There was a problem hiding this comment.
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.
-
TRC10 security checks fail through the documented default proxy.
requireTokenIdentifieraccepts numeric TRC10 IDs and the skill advertisestoken <token-id-or-contract>, but with noTRONSCAN_API_KEYconfigured,npm run security -- token 1002000returns HTTP 400 throughts.bankofai.io. The same ID succeeds withnpm run token -- --trc10 1002000and 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. -
Documented unknown token values become
no_known_flags. InassessToken,token_level: "0",black_list_type: 0, andincrease_total_supply: 0add no signals. TronScan defines these values as unknown/unrecognized. With otherwise benign fields the function returnsno_known_flags, even though the security attributes are unresolved. Please returnunknownwith explicit reasons for these enum values and for invalid field types. -
Partial transaction records are treated as resolved and clean.
assessTransactions({ [hash]: {} }, [hash])currently returnsno_known_flagswith an emptyunresolvedlist. Key presence marks the hash as resolved, while the requiredriskToken,zeroTransfer,riskAddress,sameTailAttach, andriskTransactionfields are never validated. A missing, null, or wrongly typed required field should make that hash unknown, and this case needs an offline test. -
The other assessors check property presence but not response validity.
hasAllOwnFieldsaccepts required fields whose values arenullor the wrong type. For example, four null account flags andcheat_url: nullboth becomeno_known_flags;multiSign: nullis reported as multi-signature disabled.assessApprovalsalso reports clean when the three counters exist but the requiredriskApprovearray is missing. Please use per-mode schema/type validation so incomplete upstream responses consistently fail closed asunknown. -
One transfer example is now invalid. The example in
scripts/transfer.jsstill runs--trc20 ... --direction 2without--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.
Summary
security.jscommand for TronScan account, token, URL, transaction, multi-signature permission, and token-approval security signals.unknownrather than a clean result.developas requested in feat(tronscan): add security checks and validation #71.Branch routing
developfromfeature/*.mainPR comes only fromrelease_*orhotfix/*.develop.Skill checks
SKILL.mdfiles have valid YAML frontmatter.Verification
Commands run from
tronscan-skill/:Results:
git diff --checkpasseddevelop: one commit ahead, zero behindRelease workflow
SKILL.md,package.json, andpackage-lock.jsonat the current1.0.0version.Unreleasedinstead of creating a dated1.1.0release entry.private: truetodevelopyet. If it lands before this PR, that field will be preserved while synchronizing the branch.Security
no_known_flagsis documented as not being proof that a target is safe.Previous PR
This replaces #71 following the maintainer-requested branch and release-workflow migration.