Conversation
bash /tmp/v090/audit.sh . → verdict: COMPLIANT (all 7 dimensions zero). Co-authored-by: Codex <noreply@openai.com> Co-Authored-By: Virgil <virgil@lethean.io>
📝 WalkthroughWalkthroughUpdates the Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 0/5 reviews remaining, refill in 50 minutes and 7 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
error_test.go (1)
50-63: Fold this into the existing closed-DB schema case.This exercises the same
createSchema-on-closed-handle path thatTestError_SQLiteCreateSchema_Badalready covers later in the file, so it adds maintenance noise without widening coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@error_test.go` around lines 50 - 63, Remove the redundant TestError_SQLiteInitErrors_Bad test and fold its assertions into the existing TestError_SQLiteCreateSchema_Bad case by reusing the same closed-DB setup (call newSQLiteStore, close the store via store.close, then call createSchema and assert it returns an error); ensure you reference and reuse the existing createSchema path and the closed handle teardown logic from TestError_SQLiteCreateSchema_Bad so no duplicate test covering the same createSchema-on-closed-db behavior remains.ax7_public_test.go (3)
665-679: Serialise thehttp.DefaultTransportswap.These tests mutate process-wide HTTP state. They are stable only while the package stays strictly serial, so one future parallel HTTP test will make them flaky. Please wrap the override in a package-level mutex or a helper that owns the swap.
Also applies to: 690-704
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ax7_public_test.go` around lines 665 - 679, This test mutates process-wide http.DefaultTransport; wrap the swap in a package-level mutex-protected helper so swaps are serialized: create a helper (e.g., withDefaultTransportSwap or runWithTransport) that locks a package-level sync.Mutex, saves oldTransport, sets http.DefaultTransport to ax7RoundTrip(...), defers restore and mutex.Unlock, and executes the test body; update TestAX7_CountTokens_Good (and the similar test around the other referenced lines) to call this helper instead of directly assigning http.DefaultTransport, referencing ax7RoundTrip and ratelimit.CountTokens within the helper's closure.
480-488: Replace this with a stats-specific edge case.This is effectively the same reset→zero-stats path already covered by
TestAX7_RateLimiter_Reset_Good, so it duplicates coverage without testingStatsmore directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ax7_public_test.go` around lines 480 - 488, Replace the duplicate reset-zero test with a stats-specific edge case: rename or create TestAX7_RateLimiter_Stats_EdgeCase using ax7Limiter(), and assert Stats("model-a") behavior when there was no prior activity (or when querying a non-existent model) — call rl.Stats("model-a") and use AssertEqual on expected zero values for RPD/TPM (or the expected sentinel behavior) instead of doing rl.RecordUsage + rl.Reset; ensure you still reference the rl := ax7Limiter(t), rl.Stats("model-a"), and AssertEqual checks so the test targets Stats directly rather than reset logic.
226-233: This case does not currently prove that pruning happened.
ax7Limiter(t)starts empty, sorl.AllStats()is already empty beforeBackgroundPrune()does anything. As written, this only checks thatstop()is safe. Seed observable stale state or assert on a pruner-only side effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ax7_public_test.go` around lines 226 - 233, The test currently only verifies stop() is safe because ax7Limiter() is empty; modify TestAX7_RateLimiter_BackgroundPrune_Good to seed an observable stale stat before calling BackgroundPrune: create a rate limiter entry via ax7Limiter(t).Allow(...) or rl.NewBucket/... to ensure rl.AllStats() contains an entry, then set that entry's LastSeen or timestamp to older than the prune threshold (e.g., now minus >10ms) so the background pruner should remove it; call rl.BackgroundPrune(10*time.Millisecond), wait, AssertNotPanics(t, stop) and then AssertEmpty(t, rl.AllStats()) to prove pruning occurred. Use the existing symbols TestAX7_RateLimiter_BackgroundPrune_Good, ax7Limiter, BackgroundPrune, AllStats, and stop to locate and change the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ax7_public_test.go`:
- Around line 563-570: The test(s) that call ratelimit.NewWithSQLite (e.g.,
TestAX7_NewWithSQLite_Ugly and the other NewWithSQLite/NewWithSQLiteConfig
cases) must call defer rl.Close() immediately after creating rl to avoid leaking
the DB handle on early failures; add a defer rl.Close() right after each
ratelimit.NewWithSQLite(...) / ratelimit.NewWithSQLiteConfig(...) return and you
may still keep the explicit rl.Close() later in the test for post-close
assertions since Close is idempotent.
---
Nitpick comments:
In `@ax7_public_test.go`:
- Around line 665-679: This test mutates process-wide http.DefaultTransport;
wrap the swap in a package-level mutex-protected helper so swaps are serialized:
create a helper (e.g., withDefaultTransportSwap or runWithTransport) that locks
a package-level sync.Mutex, saves oldTransport, sets http.DefaultTransport to
ax7RoundTrip(...), defers restore and mutex.Unlock, and executes the test body;
update TestAX7_CountTokens_Good (and the similar test around the other
referenced lines) to call this helper instead of directly assigning
http.DefaultTransport, referencing ax7RoundTrip and ratelimit.CountTokens within
the helper's closure.
- Around line 480-488: Replace the duplicate reset-zero test with a
stats-specific edge case: rename or create TestAX7_RateLimiter_Stats_EdgeCase
using ax7Limiter(), and assert Stats("model-a") behavior when there was no prior
activity (or when querying a non-existent model) — call rl.Stats("model-a") and
use AssertEqual on expected zero values for RPD/TPM (or the expected sentinel
behavior) instead of doing rl.RecordUsage + rl.Reset; ensure you still reference
the rl := ax7Limiter(t), rl.Stats("model-a"), and AssertEqual checks so the test
targets Stats directly rather than reset logic.
- Around line 226-233: The test currently only verifies stop() is safe because
ax7Limiter() is empty; modify TestAX7_RateLimiter_BackgroundPrune_Good to seed
an observable stale stat before calling BackgroundPrune: create a rate limiter
entry via ax7Limiter(t).Allow(...) or rl.NewBucket/... to ensure rl.AllStats()
contains an entry, then set that entry's LastSeen or timestamp to older than the
prune threshold (e.g., now minus >10ms) so the background pruner should remove
it; call rl.BackgroundPrune(10*time.Millisecond), wait, AssertNotPanics(t, stop)
and then AssertEmpty(t, rl.AllStats()) to prove pruning occurred. Use the
existing symbols TestAX7_RateLimiter_BackgroundPrune_Good, ax7Limiter,
BackgroundPrune, AllStats, and stop to locate and change the test.
In `@error_test.go`:
- Around line 50-63: Remove the redundant TestError_SQLiteInitErrors_Bad test
and fold its assertions into the existing TestError_SQLiteCreateSchema_Bad case
by reusing the same closed-DB setup (call newSQLiteStore, close the store via
store.close, then call createSchema and assert it returns an error); ensure you
reference and reuse the existing createSchema path and the closed handle
teardown logic from TestError_SQLiteCreateSchema_Bad so no duplicate test
covering the same createSchema-on-closed-db behavior remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ea7eb90-40d3-4418-b292-84efc02be7b6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
ax7_public_test.goerror_test.gogo.modratelimit.goratelimit_test.gosqlite.go
| func TestAX7_NewWithSQLite_Ugly(t *T) { | ||
| path := Path(t.TempDir(), "limits with spaces.db") | ||
| rl, err := ratelimit.NewWithSQLite(path) | ||
| RequireNoError(t, err) | ||
| rl.RecordUsage("gemini-3-pro-preview", 1, 1) | ||
|
|
||
| AssertNoError(t, rl.Persist()) | ||
| AssertNoError(t, rl.Close()) |
There was a problem hiding this comment.
Add defer rl.Close() immediately after each SQLite constructor.
These cases rely on a later manual Close(), so an earlier failure leaks the handle and skips the repo rule. Because Close is idempotent here, you can still close explicitly inside the test body when you want to assert post-close behaviour.
As per coding guidelines, "Always use defer rl.Close() when using SQLite backend constructors (NewWithSQLite() / NewWithSQLiteConfig())".
Also applies to: 613-619, 621-626
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ax7_public_test.go` around lines 563 - 570, The test(s) that call
ratelimit.NewWithSQLite (e.g., TestAX7_NewWithSQLite_Ugly and the other
NewWithSQLite/NewWithSQLiteConfig cases) must call defer rl.Close() immediately
after creating rl to avoid leaking the DB handle on early failures; add a defer
rl.Close() right after each ratelimit.NewWithSQLite(...) /
ratelimit.NewWithSQLiteConfig(...) return and you may still keep the explicit
rl.Close() later in the test for post-close assertions since Close is
idempotent.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.woodpecker.yml:
- Line 3: The CI is currently referencing an admin-scoped Sonar token
(sonar_token); replace it with a least-privilege project-scoped Sonar analysis
token, rotate the existing admin secret, and update the pipeline to use the new
project-scoped secret variable (replace usages of sonar_token and any references
on lines 34-35) so only the analysis token is stored in secrets; ensure the
secret name is updated consistently in .woodpecker.yml and in any pipeline steps
that pass the token to the Sonar scanner.
- Line 11: Replace the mutable CI image tag "image:
golangci/golangci-lint:latest-alpine" with a pinned version and digest (for
example use a specific semver tag and the corresponding sha256 digest) to ensure
reproducible builds; update every occurrence (including the other instance
noted) so both "image: golangci/golangci-lint:latest-alpine" entries are changed
to the chosen immutable form (e.g. "golangci/golangci-lint:vX.Y.Z@sha256:...")
and verify the digest matches the upstream registry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7485258-b496-45ee-86e9-f2de356d72ab
📒 Files selected for processing (2)
.woodpecker.ymlsonar-project.properties
✅ Files skipped from review due to trivial changes (1)
- sonar-project.properties
| @@ -0,0 +1,37 @@ | |||
| # Woodpecker CI pipeline. | |||
| # Server: ci.lthn.sh. Lint + sonar in parallel, both depend only on clone. | |||
| # sonar_token is admin-scoped on the Woodpecker server. | |||
There was a problem hiding this comment.
Use a least-privilege Sonar token instead of an admin-scoped secret.
The pipeline comment indicates the token is admin-scoped. For CI scanning, use a project-scoped analysis token and rotate the current secret.
Also applies to: 34-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.woodpecker.yml at line 3, The CI is currently referencing an admin-scoped
Sonar token (sonar_token); replace it with a least-privilege project-scoped
Sonar analysis token, rotate the existing admin secret, and update the pipeline
to use the new project-scoped secret variable (replace usages of sonar_token and
any references on lines 34-35) so only the analysis token is stored in secrets;
ensure the secret name is updated consistently in .woodpecker.yml and in any
pipeline steps that pass the token to the Sonar scanner.
|
|
||
| steps: | ||
| - name: golangci-lint | ||
| image: golangci/golangci-lint:latest-alpine |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the current stable Docker tags and immutable digests for golangci/golangci-lintandsonarsource/sonar-scanner-cli that are recommended for CI pinning?
💡 Result:
For golangci/golangci-lint, the current stable tags are v2.11.4 (full image) and v2.11.4-alpine (smaller image), matching the latest release v2.11.4. 'latest' also points to v2.11.4. Recommended for CI pinning: Use specific version tags like 'v2.11.4' or 'v2.11.4-alpine' as official docs emphasize installing specific versions for reproducible CI. Digests (linux/amd64): - v2.11.4: sha256:3cb05e6803ae... - v2.11.4-alpine: sha256:764c339d1afa... For sonarsource/sonar-scanner-cli, the current stable tags are 12.1.0.3225_8.0.1 (full), 12.1 (alias), and 'latest' all pointing to the same image, updated 6 days ago. Recommended for CI pinning: Use specific version tags like '12.1' or '12.1.0.3225_8.0.1' following Docker best practices and official usage examples. Digest (linux/amd64): sha256:a429d20d22c8...
Citations:
- 1: https://hub.docker.com/r/golangci/golangci-lint/tags
- 2: https://github.com/golangci/golangci-lint/tags
- 3: https://hub.docker.com/r/golangci/golangci-lint
- 4: https://hub.docker.com/r/sonarsource/sonar-scanner-cli/tags
- 5: https://github.com/SonarSource/sonar-scanner-cli-docker
- 6: https://github.com/SonarSource/sonar-scanner-cli-docker/tags
- 7: https://docs.sonarsource.com/sonarqube-server/analyzing-source-code/scanners/sonarscanner
- 8: https://golangci-lint.run/docs/welcome/install/ci
Pin CI images to immutable versions and digests.
Using latest* tags makes builds non-deterministic and vulnerable to upstream changes breaking compliance and reproducibility. Stable versions with digests are available and recommended by both projects.
Suggested hardening
- image: golangci/golangci-lint:latest-alpine
+ image: golangci/golangci-lint:v2.11.4-alpine@sha256:764c339d1afa...
...
- image: sonarsource/sonar-scanner-cli:latest
+ image: sonarsource/sonar-scanner-cli:12.1@sha256:a429d20d22c8...Also applies to: 30-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.woodpecker.yml at line 11, Replace the mutable CI image tag "image:
golangci/golangci-lint:latest-alpine" with a pinned version and digest (for
example use a specific semver tag and the corresponding sha256 digest) to ensure
reproducible builds; update every occurrence (including the other instance
noted) so both "image: golangci/golangci-lint:latest-alpine" entries are changed
to the chosen immutable form (e.g. "golangci/golangci-lint:vX.Y.Z@sha256:...")
and verify the digest matches the upstream registry.
1 participant
Brings this repo to verdict: COMPLIANT against the v0.9.0 audit (bash /tmp/v090/audit.sh . — all 7 dimensions zero).
🤖 Generated with Claude Code + Codex
Co-Authored-By: Codex noreply@openai.com
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
Chores
Tests