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>
📝 WalkthroughWalkthroughThe pull request consolidates dependencies by migrating from split 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. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Review rate limit: 1/5 review remaining, refill in 42 minutes and 39 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: 9
🧹 Nitpick comments (2)
types/ax7_generated_test.go (1)
8-141: ⚡ Quick winStrengthen JSON marshal assertions beyond substring matching
Several marshal tests (for example Line 11, Line 17, Line 89, Line 128) only check substrings, which can pass on malformed or unexpected payload shapes. Prefer decoding into
map[string]anyand asserting exact key/value expectations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/ax7_generated_test.go` around lines 8 - 141, Many MarshalJSON tests assert only via substring matching which is brittle; update tests like TestAX7_CreateFileOptions_MarshalJSON_Good/Bad/Ugly, TestAX7_UpdateFileOptions_MarshalJSON_*, TestAX7_ChangeFileOperation_MarshalJSON_*, and TestAX7_MergePullRequestOption_MarshalJSON_* to decode the produced JSON into map[string]any (or a concrete struct) and assert exact keys and values (e.g., check presence and exact value of "content" or "content_base64", and "Do"/MergeStyle) instead of using AssertContains, ensuring the marshaled shape and values match expectations..woodpecker.yml (1)
11-11: ⚡ Quick winPin CI images to immutable versions/digests.
latest-alpineandlatestmake builds non-reproducible and can break without code changes. Pin explicit versions (ideally with digests).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, The CI image is using a mutable tag "image: golangci/golangci-lint:latest-alpine" which makes builds non-reproducible; update that entry to a pinned, immutable tag or digest (for example a specific version tag or sha256 digest) and do the same for the other occurrence mentioned (30-30). Replace the mutable "latest-alpine" reference with a concrete version/digest for golangci/golangci-lint so the pipeline uses a fixed, reproducible image.
🤖 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 config currently documents/uses an admin-scoped secret for
Sonar ("sonar_token") which is over-privileged; replace the admin-scoped token
with a project-scoped Sonar token, rotate the existing secret value in the
secrets manager, update the .woodpecker.yml reference/comment (and any other
mentions on lines 34-35) to indicate it is project-scoped, and ensure the
pipeline uses the rotated secret name (e.g., SONAR_PROJECT_TOKEN) so no admin
credentials remain in CI.
In `@client.go`:
- Around line 465-467: The current code builds the full multipart payload into a
bytes.Buffer named "body" via multipart.NewWriter which can OOM for large
uploads; replace this with an io.Pipe so the multipart is streamed: create pr,
pw := io.Pipe(), use multipart.NewWriter(pw) (keep the variable name "writer" if
desired), spawn a goroutine that iterates the "fields" and writes parts to
"writer", then close writer and pw when done or on error, and use pr as the http
request body (http.NewRequest) while setting the Content-Type header to
writer.FormDataContentType(); this streams data incrementally and avoids
buffering the entire payload in memory.
In `@cmd/forgegen/generator_test.go`:
- Line 24: Tests in generator_test.go are swallowing filesystem errors by
assigning os.ReadDir (and similar file reads) errors to `_`, which can hide I/O
failures; change all occurrences that do `entries, _ := os.ReadDir(...)` or
similar to capture the error and fail the test immediately (e.g., `entries, err
:= os.ReadDir(...); if err != nil { t.Fatalf("ReadDir failed: %v", err) }`) and
likewise use t.Fatal/t.Fatalf for os.ReadFile and related reads so failures
surface instead of producing downstream assertion errors.
- Line 4: Tests in generator_test.go are using os.ReadDir/os.ReadFile and
discarding errors; replace all direct os file I/O calls (os.ReadDir,
os.ReadFile) with the repository helper coreio (imported as coreio
"dappco.re/go/core/io") and propagate/check returned errors instead of using the
blank identifier. Specifically, update every occurrence referenced in the
comment to call coreio.ReadDir/coreio.ReadFile, add the coreio import, and
replace patterns like _, _ = ... with proper err handling (t.Fatalf/t.Errorf or
require.NoError as appropriate) within the test functions in generator_test.go
so failures surface correctly.
In `@cmd/forgegen/generator.go`:
- Around line 222-223: Replace direct stdlib file ops with the project's coreio
helpers: in the Generate function (where os.MkdirAll(outDir, 0755) is used)
switch to coreio.MkdirAll and where os.WriteFile is used (around the block at
lines ~413–414) switch to coreio.WriteFile (or the appropriate coreio helper for
writing files), and add the import coreio "dappco.re/go/core/io" to the file;
ensure permissions/flags are passed per coreio API and remove usages of
os.MkdirAll/os.WriteFile so all generated-file I/O uses coreio.
In `@cmd/forgegen/main_test.go`:
- Line 4: The test currently uses os.ReadDir which violates the repo rule;
replace calls to os.ReadDir with the coreio directory helpers from the project's
go-io abstraction (imported as coreio "dappco.re/go/core/io") so tests use
coreio.ReadDir/DirEntries (or equivalent) instead of os.ReadDir; update the
import to include coreio and change any os.ReadDir usage in
cmd/forgegen/main_test.go to the corresponding coreio helper while removing the
os import if no longer needed.
In `@cmd/forgegen/parser.go`:
- Line 6: Replace the direct stdlib file I/O call with the project's go-io
helper: import coreio "dappco.re/go/core/io" (remove the unused "os" import) and
change the os.ReadFile(...) call at the spec-loading site in parser.go (around
the code that reads the spec at line ~116) to use coreio.ReadFile(...) and
handle the returned bytes/error exactly as before; ensure any error messages
remain unchanged and that the import alias coreio is used so the code follows
the repository I/O contract.
In `@config_test.go`:
- Around line 65-66: Replace direct os.* file operations in config_test.go with
the repository's go-io helpers: import coreio "dappco.re/go/core/io" and use
coreio.MkdirAll, coreio.WriteFile, and coreio.ReadFile (or the specific coreio
helpers used elsewhere in the project) in place of os.MkdirAll, os.WriteFile,
and os.ReadFile for the test setup/readback around usages that reference cfgPath
and the subsequent file write/read assertions (occurrences at the shown blocks
and the other ranges 75-76, 120-124, 176-177); remove the os calls and adjust
imports accordingly so tests use the coreio abstractions consistently.
In `@config.go`:
- Around line 50-51: Replace direct os file ops with the coreio helpers: use
coreio.ReadFile(path) instead of os.ReadFile, coreio.WriteFile(path, data, perm)
instead of os.WriteFile, and coreio.MkdirAll(dir, perm) instead of os.MkdirAll;
add the coreio import alias if missing. For the write path that currently
returns a raw error (the os.WriteFile replacement), wrap the error with the
repository’s error-wrapping style (e.g. fmt.Errorf("writing config %s: %w",
path, err)) so the failure includes operation context. Update all occurrences
around the ReadFile/WriteFile/MkdirAll calls in config.go to follow this
pattern.
---
Nitpick comments:
In @.woodpecker.yml:
- Line 11: The CI image is using a mutable tag "image:
golangci/golangci-lint:latest-alpine" which makes builds non-reproducible;
update that entry to a pinned, immutable tag or digest (for example a specific
version tag or sha256 digest) and do the same for the other occurrence mentioned
(30-30). Replace the mutable "latest-alpine" reference with a concrete
version/digest for golangci/golangci-lint so the pipeline uses a fixed,
reproducible image.
In `@types/ax7_generated_test.go`:
- Around line 8-141: Many MarshalJSON tests assert only via substring matching
which is brittle; update tests like
TestAX7_CreateFileOptions_MarshalJSON_Good/Bad/Ugly,
TestAX7_UpdateFileOptions_MarshalJSON_*,
TestAX7_ChangeFileOperation_MarshalJSON_*, and
TestAX7_MergePullRequestOption_MarshalJSON_* to decode the produced JSON into
map[string]any (or a concrete struct) and assert exact keys and values (e.g.,
check presence and exact value of "content" or "content_base64", and
"Do"/MergeStyle) instead of using AssertContains, ensuring the marshaled shape
and values match expectations.
🪄 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: 74d0a202-4049-4bf6-8bea-4c2adce3d3a7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (21)
.woodpecker.ymladmin.goax7_generated_test.goclient.goclient_test.gocmd/forgegen/ax7_generated_test.gocmd/forgegen/generator.gocmd/forgegen/generator_test.gocmd/forgegen/helpers.gocmd/forgegen/main.gocmd/forgegen/main_test.gocmd/forgegen/parser.goconfig.goconfig_test.gogo.modhelpers.gopagination.goparams.goresource.gosonar-project.propertiestypes/ax7_generated_test.go
| @@ -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.
Do not run Sonar with an admin-scoped token.
Line 3 documents admin scope for the CI secret; this is over-privileged for scanning and increases blast radius. Use a project-scoped 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 config currently documents/uses an
admin-scoped secret for Sonar ("sonar_token") which is over-privileged; replace
the admin-scoped token with a project-scoped Sonar token, rotate the existing
secret value in the secrets manager, update the .woodpecker.yml
reference/comment (and any other mentions on lines 34-35) to indicate it is
project-scoped, and ensure the pipeline uses the rotated secret name (e.g.,
SONAR_PROJECT_TOKEN) so no admin credentials remain in CI.
| var body bytes.Buffer | ||
| writer := multipart.NewWriter(&body) | ||
| for key, value := range fields { |
There was a problem hiding this comment.
Avoid buffering the entire multipart payload in memory.
Line 465 and Line 486 now build the full upload in a bytes.Buffer before request dispatch. That can cause large memory spikes/OOMs for sizeable assets or concurrent uploads. Please stream via io.Pipe so data is produced incrementally.
Suggested direction (streamed multipart)
- var body bytes.Buffer
- writer := multipart.NewWriter(&body)
+ pr, pw := goio.Pipe()
+ writer := multipart.NewWriter(pw)
+ writeErr := make(chan error, 1)
+ go func() {
+ defer close(writeErr)
+ defer pw.Close()
+ for key, value := range fields {
+ if err := writer.WriteField(key, value); err != nil {
+ _ = pw.CloseWithError(err)
+ writeErr <- err
+ return
+ }
+ }
+ if fieldName != "" {
+ part, err := writer.CreateFormFile(fieldName, fileName)
+ if err != nil {
+ _ = pw.CloseWithError(err)
+ writeErr <- err
+ return
+ }
+ if content != nil {
+ if _, err := goio.Copy(part, content); err != nil {
+ _ = pw.CloseWithError(err)
+ writeErr <- err
+ return
+ }
+ }
+ }
+ if err := writer.Close(); err != nil {
+ _ = pw.CloseWithError(err)
+ writeErr <- err
+ return
+ }
+ }()
- req, err := http.NewRequestWithContext(ctx, http.MethodPost, target, &body)
+ req, err := http.NewRequestWithContext(ctx, http.MethodPost, target, pr)Also applies to: 486-486
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client.go` around lines 465 - 467, The current code builds the full multipart
payload into a bytes.Buffer named "body" via multipart.NewWriter which can OOM
for large uploads; replace this with an io.Pipe so the multipart is streamed:
create pr, pw := io.Pipe(), use multipart.NewWriter(pw) (keep the variable name
"writer" if desired), spawn a goroutine that iterates the "fields" and writes
parts to "writer", then close writer and pw when done or on error, and use pr as
the http request body (http.NewRequest) while setting the Content-Type header to
writer.FormDataContentType(); this streams data incrementally and avoids
buffering the entire payload in memory.
| package main | ||
|
|
||
| import ( | ||
| "os" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check file size first
wc -l cmd/forgegen/generator_test.goRepository: dAppCore/go-forge
Length of output: 96
🏁 Script executed:
# Read the file with line numbers to verify imports and usage at specified lines
cat -n cmd/forgegen/generator_test.go | head -30Repository: dAppCore/go-forge
Length of output: 777
🏁 Script executed:
# Check lines around 24, 50, 54
sed -n '20,60p' cmd/forgegen/generator_test.go | cat -nRepository: dAppCore/go-forge
Length of output: 1209
🏁 Script executed:
# Check lines around 89, 91, 131, 133
sed -n '85,140p' cmd/forgegen/generator_test.go | cat -nRepository: dAppCore/go-forge
Length of output: 1802
🏁 Script executed:
# Check lines around 155, 159, 193, 196
sed -n '150,200p' cmd/forgegen/generator_test.go | cat -nRepository: dAppCore/go-forge
Length of output: 1824
🏁 Script executed:
# Check lines around 230, 248
sed -n '225,255p' cmd/forgegen/generator_test.go | cat -nRepository: dAppCore/go-forge
Length of output: 1198
🏁 Script executed:
# Also check if coreio is used anywhere in the file
grep -n "coreio" cmd/forgegen/generator_test.go || echo "No coreio imports found"Repository: dAppCore/go-forge
Length of output: 85
🏁 Script executed:
# Check what coreio package provides
find . -name "*.go" -type f | xargs grep -l "dappco.re/go/core/io" | head -5Repository: dAppCore/go-forge
Length of output: 43
Replace direct os file I/O with coreio in these tests.
Lines 4, 24, 50, 54, 89, 91, 131, 133, 155, 159, 193, 196, 230, and 248 use os.ReadDir() and os.ReadFile(), which violates the repository guideline requiring coreio "dappco.re/go/core/io" for all Go file I/O operations. Error return values are also being silently discarded with _, which prevents proper error handling in tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/forgegen/generator_test.go` at line 4, Tests in generator_test.go are
using os.ReadDir/os.ReadFile and discarding errors; replace all direct os file
I/O calls (os.ReadDir, os.ReadFile) with the repository helper coreio (imported
as coreio "dappco.re/go/core/io") and propagate/check returned errors instead of
using the blank identifier. Specifically, update every occurrence referenced in
the comment to call coreio.ReadDir/coreio.ReadFile, add the coreio import, and
replace patterns like _, _ = ... with proper err handling (t.Fatalf/t.Errorf or
require.NoError as appropriate) within the test functions in generator_test.go
so failures surface correctly.
| } | ||
|
|
||
| entries, _ := coreio.Local.List(outDir) | ||
| entries, _ := os.ReadDir(outDir) |
There was a problem hiding this comment.
Stop swallowing filesystem errors in test reads.
At these sites, ignored errors (_) can convert I/O failures into misleading assertion failures; fail immediately with t.Fatal/t.Fatalf when directory or file reads fail.
Also applies to: 50-50, 89-91, 131-133, 155-160, 193-198, 227-233, 247-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/forgegen/generator_test.go` at line 24, Tests in generator_test.go are
swallowing filesystem errors by assigning os.ReadDir (and similar file reads)
errors to `_`, which can hide I/O failures; change all occurrences that do
`entries, _ := os.ReadDir(...)` or similar to capture the error and fail the
test immediately (e.g., `entries, err := os.ReadDir(...); if err != nil {
t.Fatalf("ReadDir failed: %v", err) }`) and likewise use t.Fatal/t.Fatalf for
os.ReadFile and related reads so failures surface instead of producing
downstream assertion errors.
| if err := os.MkdirAll(outDir, 0755); err != nil { | ||
| return core.E("Generate", "create output directory", err) |
There was a problem hiding this comment.
Generated file output must use coreio, not os.MkdirAll/os.WriteFile
Line 222 and Line 413 use direct stdlib file operations. Please switch these back to the coreio helpers to preserve the project’s enforced I/O abstraction.
As per coding guidelines, **/*.go: “Use file I/O with go-io (imported as coreio "dappco.re/go/core/io"), never os.ReadFile/os.WriteFile/os.MkdirAll.”
Also applies to: 413-414
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/forgegen/generator.go` around lines 222 - 223, Replace direct stdlib file
ops with the project's coreio helpers: in the Generate function (where
os.MkdirAll(outDir, 0755) is used) switch to coreio.MkdirAll and where
os.WriteFile is used (around the block at lines ~413–414) switch to
coreio.WriteFile (or the appropriate coreio helper for writing files), and add
the import coreio "dappco.re/go/core/io" to the file; ensure permissions/flags
are passed per coreio API and remove usages of os.MkdirAll/os.WriteFile so all
generated-file I/O uses coreio.
| package main | ||
|
|
||
| import ( | ||
| "os" |
There was a problem hiding this comment.
Use coreio directory listing helpers instead of os.ReadDir
Line 16 introduces direct stdlib file I/O in test code. Please switch this back to the project’s coreio abstraction to stay compliant with the repository rule set.
As per coding guidelines, **/*.go: “Use file I/O with go-io (imported as coreio "dappco.re/go/core/io"), never os.ReadFile/os.WriteFile/os.MkdirAll.”
Also applies to: 16-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/forgegen/main_test.go` at line 4, The test currently uses os.ReadDir
which violates the repo rule; replace calls to os.ReadDir with the coreio
directory helpers from the project's go-io abstraction (imported as coreio
"dappco.re/go/core/io") so tests use coreio.ReadDir/DirEntries (or equivalent)
instead of os.ReadDir; update the import to include coreio and change any
os.ReadDir usage in cmd/forgegen/main_test.go to the corresponding coreio helper
while removing the os import if no longer needed.
| import ( | ||
| "cmp" | ||
| json "github.com/goccy/go-json" | ||
| "os" |
There was a problem hiding this comment.
Revert spec loading to coreio instead of os.ReadFile
Line 116 uses direct stdlib file I/O and breaks the repository’s I/O contract. Please restore the coreio read helper here.
As per coding guidelines, **/*.go: “Use file I/O with go-io (imported as coreio "dappco.re/go/core/io"), never os.ReadFile/os.WriteFile/os.MkdirAll.”
Also applies to: 116-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/forgegen/parser.go` at line 6, Replace the direct stdlib file I/O call
with the project's go-io helper: import coreio "dappco.re/go/core/io" (remove
the unused "os" import) and change the os.ReadFile(...) call at the spec-loading
site in parser.go (around the code that reads the spec at line ~116) to use
coreio.ReadFile(...) and handle the returned bytes/error exactly as before;
ensure any error messages remain unchanged and that the import alias coreio is
used so the code follows the repository I/O contract.
| if err := os.MkdirAll(filepath.Dir(cfgPath), 0700); err != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
Test filesystem setup/readback should use coreio abstractions
These segments replaced project-standard file helpers with os.MkdirAll, os.WriteFile, and os.ReadFile. Please revert to coreio helpers to keep test code aligned with repository compliance rules.
As per coding guidelines, **/*.go: “Use file I/O with go-io (imported as coreio "dappco.re/go/core/io"), never os.ReadFile/os.WriteFile/os.MkdirAll.”
Also applies to: 75-76, 120-124, 176-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config_test.go` around lines 65 - 66, Replace direct os.* file operations in
config_test.go with the repository's go-io helpers: import coreio
"dappco.re/go/core/io" and use coreio.MkdirAll, coreio.WriteFile, and
coreio.ReadFile (or the specific coreio helpers used elsewhere in the project)
in place of os.MkdirAll, os.WriteFile, and os.ReadFile for the test
setup/readback around usages that reference cfgPath and the subsequent file
write/read assertions (occurrences at the shown blocks and the other ranges
75-76, 120-124, 176-177); remove the os calls and adjust imports accordingly so
tests use the coreio abstractions consistently.
| data, err := os.ReadFile(path) | ||
| if err != nil { |
There was a problem hiding this comment.
Replace direct os.* file operations and keep write errors wrapped
Line 50, Line 76, and Line 83 bypass the repository I/O abstraction, and Line 83 now returns a raw error without operation context. Please switch these back to the coreio file helpers and wrap the write failure with your existing error wrapper style for consistent diagnostics.
As per coding guidelines, **/*.go: “Use file I/O with go-io (imported as coreio "dappco.re/go/core/io"), never os.ReadFile/os.WriteFile/os.MkdirAll.”
Also applies to: 76-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config.go` around lines 50 - 51, Replace direct os file ops with the coreio
helpers: use coreio.ReadFile(path) instead of os.ReadFile,
coreio.WriteFile(path, data, perm) instead of os.WriteFile, and
coreio.MkdirAll(dir, perm) instead of os.MkdirAll; add the coreio import alias
if missing. For the write path that currently returns a raw error (the
os.WriteFile replacement), wrap the error with the repository’s error-wrapping
style (e.g. fmt.Errorf("writing config %s: %w", path, err)) so the failure
includes operation context. Update all occurrences around the
ReadFile/WriteFile/MkdirAll calls in config.go to follow this pattern.
1 participant
Brings this repo to verdict: COMPLIANT against the v0.9.0 audit (799 entry-state findings).
🤖 Generated with Claude Code + Codex
Co-Authored-By: Codex noreply@openai.com
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores