Skip to content

harden(ci): wire CI workflow, skip solana stub test when solders missing, add SECURITY#35

Open
RTHYMS wants to merge 1 commit into
kcolbchain:mainfrom
RTHYMS:contrib/harden-ci
Open

harden(ci): wire CI workflow, skip solana stub test when solders missing, add SECURITY#35
RTHYMS wants to merge 1 commit into
kcolbchain:mainfrom
RTHYMS:contrib/harden-ci

Conversation

@RTHYMS

@RTHYMS RTHYMS commented Jun 10, 2026

Copy link
Copy Markdown

Summary

`test_generate_new_wallet` fails in any environment without `solders` on the path:

```
AssertionError: assert 11 > 30

  • where 11 = len('STUB_PUBKEY')
  • where 'STUB_PUBKEY' = SolanaWallet(...).pubkey
    ```

The `SolanaConnector` deliberately falls back to a stub wallet (`STUB_PUBKEY`, 11 chars) when `solders` isn't importable — see the `solders not installed -- using stub wallet` log. The test asserts `len(pubkey) > 30` (a real base58 pubkey), but that contract only holds when solders is actually installed. So the test fails on any CI image without the (optional, native) solders dependency.

Fix

Detect solders at import-time and skip the test cleanly when unavailable:

```python
try:
import solders # noqa: F401
HAS_SOLDERS = True
except ImportError:
HAS_SOLDERS = False

class TestSolanaWallet:
@pytest.mark.skipif(not HAS_SOLDERS, reason="solders not installed; connector falls back to stub")
def test_generate_new_wallet(self):
...
```

Preserves the regression intent (when solders IS present, the assertion still pins real-base58 behaviour) while making CI robust.

Result

```
99 passed, 1 skipped in 1.12s
```

(was 99 passed, 1 failed)

Drive-by

  • CI workflow + badge — repo had tests but no CI.
  • SECURITY.md.

Note (out of scope)

Repo has 315 `datetime.utcnow()` deprecation warnings on Python 3.14. `utcnow()` is gone in a future Python; recommend a coordinated `datetime.now(timezone.utc)` sweep (same pattern as scout PR #17). Happy to follow up in a separate PR.

…rs missing, add SECURITY

- Add .github/workflows/ci.yml — pytest on Python 3.11 and 3.12. The repo
  shipped tests/ but no automation; CI is the missing surface.
- Skip test_generate_new_wallet when `solders` is not installed. The
  connector falls back to a stub returning the literal "STUB_PUBKEY"
  (11 chars), but the test asserts len > 30 for a real base58 pubkey,
  causing a false failure in any env where solders isn't on the path.
  pytest.skipif keeps the regression intent while making CI robust.
- Add .github/SECURITY.md so vulnerability disclosure has a documented
  channel.
@abhicris

Copy link
Copy Markdown
Contributor

Welcome to kcolbchain, @RTHYMS — glad you're here. 🌱

Here's what happens from this PR:

  1. Our automated review looks for obvious issues (tests, secrets, size) within a couple of hours.
  2. If it's clean and CI passes, we merge without back-and-forth.
  3. If we need changes, we'll leave a specific comment — not a generic nit. Push another commit and we re-review.

While you wait:

  • Run the repo's tests locally (see the repo README.md).
  • Keep the PR scoped to one concern — bigger PRs land slower.
  • Don't commit tokens or .env contents.

What happens after your first merge

Thanks for writing the code. We're building this to last.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants