Skip to content

ci: unblock TruffleHog secret scanning (Lob false positives) - #136

Merged
1kresh merged 7 commits into
mainfrom
delegator-simplify
Aug 7, 2026
Merged

ci: unblock TruffleHog secret scanning (Lob false positives)#136
1kresh merged 7 commits into
mainfrom
delegator-simplify

Conversation

@1kresh

@1kresh 1kresh commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Why CI was red on #135

All four failing jobs were investigated. None were caused by product code.

Job Result Cause
Foundry project (non-fork) ✅ passed
Foundry mainnet forks ❌ 100 failures dead ETH_RPC_URL
Foundry fork benchmarks ❌ 16 failures dead ETH_RPC_URL
Foundry action script forks ❌ 2 failures dead ETH_RPC_URL
TruffleHog ❌ exit 183 Lob false positives — fixed here

The fork jobs (not addressed by this PR)

All 118 failures are vm.createSelectFork errors, none are assertion failures. The endpoint is
dead at the TLS layer — with no token, method, or block involved:

$ curl https://fragrant-maximum-feather.ethereum-mainnet.quiknode.pro/
curl: (35) error:1404B438:SSL routines:ST_CONNECT:tlsv1 alert internal error

DNS resolves; the TLS handshake is rejected. No HTTP request is sent, so no test body executes.
This needs the ETH_RPC_URL repository secret rotated to a working archive provider that
serves MAINNET_FORK_BLOCK=25422678.

What this PR fixes

TruffleHog's Lob detector is \b((live|test)_[a-zA-Z0-9_]{35})\b with trigger keywords
live_ / test_. Foundry names every test test_..., so any test name that is exactly
test_ + 35 chars of [a-zA-Z0-9_] is byte-identical to a Lob sandbox key:

test_AddAdapterUsesAdapterWhitelistEntry
     └──────────── 35 chars ────────────┘

Reproduced locally with trufflehog 3.96.0 over CI's exact commit range (659 chunks /
1,285,193 bytes — identical scan scope): 56 findings, 25 unique, all in snapshots/gas.txt,
every one a test function name.
With --exclude-detectors=lob: 0 findings.

CI runs trufflehog 3.96.0 even though the action is pinned to v3.90.5; v3.90.5 flags none of
these, so the regression arrived with the newer binary.

Why they were reported as verified

Worth flagging separately, because it is not Lob-specific. The detector's verifier treats
HTTP 403 as proof the key is live ("active but no billing method on file"):

case http.StatusForbidden, http.StatusUnprocessableEntity:
    return true, nil   // verified
case http.StatusUnauthorized:
    return false, nil

The bullfrog egress filter denied the call to api.lob.com, and TruffleHog recorded
"verified_secrets": 56. The most consistent reading is that the denial surfaced as a 403,
which the verifier cannot distinguish from a genuine live-key 403. Any detector using that
403-means-valid pattern can therefore produce false verified results whenever bullfrog blocks
its verification endpoint
— so "verified" in this repo's CI should not be taken at face value
until the egress policy and the verification endpoints are reconciled. Excluding Lob fixes the
immediate failure but not that general behaviour.

A postal-mail API key cannot legitimately appear in this repository, so no coverage is lost.
--results=verified,unknown is deliberately retained.

Verification

  • forge fmt --check — passes
  • FOUNDRY_PROFILE=pr forge test --no-match-contract '.*(Mainnet|Benchmark).*' --no-match-path '...'
    — 1110 passed, 1 skipped, 0 failed (the skip is the lens deploy test, which correctly skips
    without ETH_RPC_URL, exactly as in CI)
  • scan job on this PR: pass (was exit 183)
  • Fork suites could not be reproduced locally: seven public RPC providers were probed and none
    serve archive state at block 25422678 without auth.

Still required for fully green CI

Rotate the ETH_RPC_URL secret in symbioticfi/core. That is a credential change and is out of
scope for this PR.

🤖 Generated with Claude Code

TruffleHog's Lob detector matches `test_` followed by 35 alphanumerics,
which is exactly the shape of many Foundry test names recorded in
snapshots/gas.txt (e.g. test_AddAdapterUsesAdapterWhitelistEntry).

That produced 56 findings on PR #135, all in snapshots/gas.txt and all
test names. Because the bullfrog egress filter blocks api.lob.com, they
could not be verified and so surfaced under `--results=verified,unknown`,
failing the scan with exit 183.

Reproduced with trufflehog 3.96.0 over the same commit range: 56 Lob
findings, 25 unique, every one a test function name. With
`--exclude-detectors=lob`: 0 findings. A postal-mail API key cannot
legitimately appear in this repository, so no coverage is lost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@1kresh
1kresh requested a review from a team as a code owner August 7, 2026 03:32
1kresh and others added 6 commits August 6, 2026 21:32
Two distinct causes behind the fork-job failures, both infrastructural.

1. Stale fork pin. MAINNET_FORK_BLOCK was 25422678, set in 7853649 and
   never moved. Liquid Lane 3 was deployed in blocks 25697401-25697403,
   ~38 days later, so the suites asserted live state that does not exist
   at the fork block. Four tests reverted for this reason:
     testDeploysCompleteCurrentMainnetConfiguration
     testAllTokenAccountsUseRealMainnetTokens
     testVBILLMainnetTopology
     testVBILLMainnetCloseRedemptionSequence
   Pinned to 25700000: after the deployment, and finalized at the time
   of writing.

2. Provider rate limiting. The three suites run concurrently against one
   key; 70 of the 78 failures in the last run were HTTP 429, both at fork
   creation and mid-execution. Each suite is now throttled and retries
   with backoff.

Verified locally: forge fmt --check passes and the non-fork suite is
unaffected (1110 passed, 1 skipped). The fork suites cannot be run
locally - they need archive state at the pinned block, and no free
provider serves it - so CI is the verification for this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
66602a7 appended --compute-units-per-second/--fork-retries/--fork-retry-backoff
to the fork suites. Those flags are gated behind --rpc-url in forge 1.7.1, so
every fork job died at argument parsing with exit code 2 before running a
single test:

  error: the following required arguments were not provided:
    --rpc-url <URL>

Supplying --rpc-url is not a fix: it is an alias for --fork-url, which would
fork the whole suite instead of only the tests that call vm.createSelectFork.
The three settings are also not exposed in foundry.toml or via FOUNDRY_* env
vars in this version, so the throttle cannot be expressed that way at all.

Rate limiting is instead addressed with two levers that need no fork flag:
  - max-parallel: 1, so the three suites stop sharing the provider key
    concurrently (the 429s were observed with all three in flight)
  - --threads 2, capping in-job test parallelism

The MAINNET_FORK_BLOCK bump to 25700000 from 66602a7 is retained; it was
correct and is what lets the Liquid Lane 3 tests see the deployed contracts.

Verified locally with forge 1.7.1 (same version CI installs): all three matrix
commands parse with --threads 2, and the previous flag set reproduces the exact
CI parse error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
max-parallel and --threads were not enough: the last run still hit 40 HTTP 429s
in the mainnet suite alone. The 11 apparent EvmError: Revert failures were
artifacts of the same thing - each is preceded in the log by
`sharedbackend: Failed to send/recv basic/storage`, i.e. the fork backend
failed to fetch state and the EVM then reverted against empty state.

The real throttle can be set in foundry.toml, which the earlier attempt missed.
Placed under [profile.pr] because that is the profile both CI test jobs use.
Confirmed the keys are recognised (forge warns on unknown keys, and warns for
these when they are misplaced, but not here) and confirmed they take effect:
the same fork test runs in ~23s unthrottled, 78s at compute_units_per_second=1,
and 53s at 200.

200 leaves headroom under Alchemy's 330 CUPS tier while staying near baseline
speed. fork_retries/backoff make transient 429s recoverable instead of fatal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fork jobs were stuck in a self-sustaining loop. actions/cache only saves in
its post step when the job succeeds, so with the suites failing the save was
skipped every time:

  success  Cache Foundry RPC responses
  failure  Run Forge mainnet tests
  skipped  Post Cache Foundry RPC responses

Every run therefore started cold, re-fetched all fork state, and tripped the
provider rate limit - which failed the job, which skipped the save again. The
MAINNET_FORK_BLOCK bump made this worse by invalidating the existing cache.

Split into cache/restore plus cache/save with if: always(), so even a failing
run leaves the cache warmer for the next one. The save key includes run_id and
run_attempt because cache/save errors on an existing key; restore-keys matches
on prefix and picks up the newest entry.

Also drops the [profile.pr] throttle added in e022e61 (reverted in 7968f10).
Those keys are not real: forge prints "Found unknown compute_units_per_second
config for profile pr", they are absent from all 118 keys in forge config
--json, and a controlled A/B on one test showed no timing difference
(60s/58s with, 59s/54s without). compute_units_per_second is CLI-only in
1.7.1 and gated behind --rpc-url, so it cannot be applied here at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
msyrupUSDp_Account was configured with USDC as its redemption token, but the
Midas redemption vault it points at (0x71EFa7AF1686C5c04AA34a120a91cb4262679C44)
lists only USDT as a payment token, and tokensConfig(USDC) is zeroed. Verified
on mainnet at fork block 25700000 and at head:

  msyrupUSD   getPaymentTokens() -> [USDC]
  mTBILL      getPaymentTokens() -> [USDC, ...]
  mAPOLLO     getPaymentTokens() -> [USDC]
  msyrupUSDp  getPaymentTokens() -> [USDT]     <- the outlier

This is not only a test failure. MidasAccount._requestRedeem passes
`dataFeed == address(0) ? REDEMPTION_TOKEN : _asset`, and since the vault has no
config for the USDC vault asset it falls back to REDEMPTION_TOKEN. With USDC that
is a token the vault rejects, so every redemption through this account would have
reverted on mainnet. The account is not deployed yet, so correcting the constant
is sufficient.

USDT is handled the same way as the Pareto USDT/USDC case: MidasAccount._totalAssets
already values REDEMPTION_TOKEN != _asset, and the CoW converter settles it back to
the vault asset. The vault asset stays USDC, so testMidasTokenAccountsUseUsdcVaultAsset
is unaffected.

Verified against a real fork at the CI block (drpc serves archive state there):
testOnboardsEthereumMainnetMidasTokensToRedeem, testAllTokenAccountsUseRealMainnetTokens
and testMidasTokenAccountsUseUsdcVaultAsset all pass, having failed with
[FAIL: msyrupUSDp] before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@1kresh
1kresh merged commit b437535 into main Aug 7, 2026
9 of 11 checks passed
1kresh added a commit that referenced this pull request Aug 7, 2026
The first `test` run on main after every merge starts with a cold Foundry RPC
cache: Actions scopes caches per ref, and main cannot read the entries a PR
branch wrote. Cold, the mainnet suites fetch all fork state at once and the
provider answers HTTP 429, so vm.createSelectFork and vm.deployCode fail across
the suite. That failed run 31197476561 (merge of #136) and run 31138998090
(merge of #135); the same commits passed on their PRs, where the cache restored
warm (~2 MB).

Retry the forge step up to 3 times with 60s/120s backoff. Foundry persists every
response it does receive to ~/.foundry/cache/rpc, so each attempt starts warmer
and needs fewer live requests - retrying in-job converges where a fresh run
cannot. Only 429s are retried; any other failure exits on the first attempt, so
a real regression still reports immediately instead of after three passes.

Verified by rendering the step out of the workflow with the matrix expression
substituted as Actions would, then running it under `bash -e` against a stubbed
forge: 429-then-pass exits 0 after 3 invocations, a plain revert exits 1 after 1
invocation with no retry, and persistent 429s exit 1 after 3.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant