solana: add 'dylint' security linting tool

[johnsaigle]

Add support for the dylint tool. Configures Cargo.toml to use lints designed to help identify issues in Solana/Anchor code.

This tool adds extra functionality to clippy to allow it to flag security issues for us:

...
warning: this Account struct is used but there is no check on its owner field
   --> programs/wormhole-governance/src/instructions/governance.rs:134:26
    |
134 |             acc.pubkey = ctx.accounts.governance.key();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(missing_owner_check)]` on by default
...

Integrating this tool could provide helpful, proactive security benefits.

I think it would be good for us to resolve these warnings or else mark them explicitly as false positives and thereby have clippy ignore them.

After that initial effort is done, we can integrate this tool into CI. We can upgrade from warnings to errors, and optionally choose to have our CI pipeline reject any unaddressed discoveries.

[kcsongor]

nice! can you actually add a CI action to this PR so we know when we're done with all the fixes?

[johnsaigle]

@kcsongor I made an attempt in the latest commit.

I don't have experience with GitHub Actions so please let me know if there's a more optimal way of adding these checks.

EDIT: Looks like it's working, but actually failing because RUSTFLAGS is set to 'Deny' for clippy warnings -- which is good! But not what we want right now. I'm working on adding a manual override to Allow warnings on only the dylint command. (This is temporary until we manually resolve/allow the existing warnings.)

EDIT2: Actually my first edit makes no sense. That will basically silence the warnings... not what we want.
I think what I can do is periodically rebase this PR as we resolve existing issues. Once those are all addressed, we can merge this as-is and have it block any new issues.

[kcsongor]

An option would be to set this up as a separate CI action as opposed to putting it into the other solana tests, so it can just be optionally ignored. That being said, these all seem false positives to me, and looking at the other lints it looks like they mostly check for stuff that's otherwise enforced by anchor already.

[johnsaigle]

Hm, you're right. I was under the impression that they were more Anchor-specific but they do appear to be more targeted toward raw Solana. And agreed that the existing warnings appear to be false positives.

I think there's some value in including the tool anyway, in case there is some change that includes a true positive. It adds some noise but in my opinion it's acceptable. It probably shouldn't block PRs so I'm happy to create a separate CI target that is non-blocking but still reports warnings.

[kcsongor]

Thinking about this a bit more -- if we do decide to keep the action (which I can see might be a good idea), we should enforce it in CI. Individual false positives can always be silenced on a case by case basis, but if they're left in then over time we'll just stop looking at the output and true positives might easily slip through. That would be very embarrassing 😅

[johnsaigle]

Cool that makes sense to me. I'll do my best to resolve all the false positives so this is as painless for others as possible

[johnsaigle]

Closing in favour of #234, which will add comments rather than linting. The dylint rules are more appropriate for a plain Solana project, not one that uses Anchor