Conversation
📝 WalkthroughWalkthroughThe changes introduce Docker container testing infrastructure by adding a testcontainers dependency, modifying entrypoint initialization to check for a genesis file instead of a marker directory, removing stdout/stderr suppression, and creating a comprehensive test suite with embedded configuration fixtures that validates the SourceHub Docker image starts correctly and the chain becomes operational. Changes
Sequence DiagramsequenceDiagram
participant Test as Test Runner
participant TC as Testcontainers
participant Docker as Docker Engine
participant Container as SourceHub Container
participant Chain as Chain/GRPC Endpoint
Test->>TC: Create container request<br/>(image, mounts, env vars)
TC->>Docker: Build/pull image
Docker->>Container: Start container
Container->>Container: Run entrypoint.sh<br/>(check genesis.json)
Container->>Chain: Initialize chain
Chain->>Chain: Start GRPC & RPC servers
Test->>TC: Expose ports<br/>(9090, 26657)
TC->>Test: Return mapped endpoints
Test->>Test: waitForChain()<br/>(10s timeout)
loop Until chain ready or timeout
Test->>Chain: probeChain()<br/>(CometBFTRPC.Block height=1)
alt Probe succeeds
Chain-->>Test: Block response
Test->>Test: Chain ready ✓
else Probe fails
Test->>Test: Wait & retry
end
end
Test->>Container: Log output
Test->>Docker: Stop container
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 (3)
docker/entrypoint.sh (1)
69-69: Consider removing the vestigial.initializedmarker.Line 69 still creates
/sourcehub/.initialized, but the initialization check now usesgenesis.jsonpresence (line 9). This marker file is no longer read and can be removed to avoid confusion.🧹 Proposed cleanup
echo "Loaded Genesis from $GENESIS_PATH" fi - touch /sourcehub/.initialized else echo "Skipping initialization: container previously initialized" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/entrypoint.sh` at line 69, Remove the obsolete marker creation: delete the statement that creates "/sourcehub/.initialized" (the touch invocation) since initialization now relies on presence of "genesis.json"; ensure no other code relies on that marker and leave logic that checks for "genesis.json" unchanged.tests/docker/test.sh (2)
15-23: Add cleanup trap and consider retry logic for robustness.If the test fails (e.g.,
jqparse error, assertion failure), the container is left running. A trap ensures cleanup. Also, a fixed 7-second sleep may cause flakiness in slower CI environments.🔧 Proposed fix with cleanup trap
+cleanup() { + docker stop sourcehub-df-test 2>/dev/null || true +} +trap cleanup EXIT + sleep 7 last_block=$(docker exec sourcehub-df-test sourcehubd status | jq .sync_info.latest_block_height) echo "Last SourceHub block $last_block" test "$last_block" -gt "0" - -docker stop sourcehub-df-test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/docker/test.sh` around lines 15 - 23, Add a cleanup trap and retry logic to make the script robust: install a trap that stops (and optionally removes) the container started for the test on EXIT/ERR to ensure docker stop sourcehub-df-test always runs, replace the fixed sleep with a retry loop that runs docker exec sourcehub-df-test sourcehubd status | jq .sync_info.latest_block_height into the last_block variable with a short sleep/backoff and a timeout, validate jq output before using test -gt to avoid parse errors, and fail with a clear message if the retry timeout is reached; reference the last_block assignment and the docker exec/sourcehub-df-test invocation to locate where to implement these changes.
1-2: Use a more portable shebang.
/usr/bin/bashmay not exist on all systems (e.g., macOS has bash at/bin/bash). Consider using#!/usr/bin/env bashfor better portability.🔧 Proposed fix
-#!/usr/bin/bash +#!/usr/bin/env bash set -e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/docker/test.sh` around lines 1 - 2, Replace the non-portable shebang in tests/docker/test.sh (currently "#!/usr/bin/bash") with a portable one using the env lookup (e.g., "#!/usr/bin/env bash") so the script finds bash across different systems; update the first line accordingly and ensure the rest of the script continues to use bash-compatible features (the following "set -e" and other bash usage need no change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/docker/test.sh`:
- Around line 4-13: The docker run in tests/docker/test.sh mounts the current
working directory with "-v .:/secrets", which breaks if the script is invoked
from a different directory; update the docker run command to compute the
script's directory and mount that absolute path instead of "." (e.g., derive the
script directory with dirname/realpath or cd+pwd and use that path in place of
"-v .:/secrets") so the container always receives the intended secret files when
running the docker run invocation that includes MNEMONIC_PATH,
CONSENSUS_KEY_PATH, COMET_NODE_KEY_PATH and GENESIS_PATH.
---
Nitpick comments:
In `@docker/entrypoint.sh`:
- Line 69: Remove the obsolete marker creation: delete the statement that
creates "/sourcehub/.initialized" (the touch invocation) since initialization
now relies on presence of "genesis.json"; ensure no other code relies on that
marker and leave logic that checks for "genesis.json" unchanged.
In `@tests/docker/test.sh`:
- Around line 15-23: Add a cleanup trap and retry logic to make the script
robust: install a trap that stops (and optionally removes) the container started
for the test on EXIT/ERR to ensure docker stop sourcehub-df-test always runs,
replace the fixed sleep with a retry loop that runs docker exec
sourcehub-df-test sourcehubd status | jq .sync_info.latest_block_height into the
last_block variable with a short sleep/backoff and a timeout, validate jq output
before using test -gt to avoid parse errors, and fail with a clear message if
the retry timeout is reached; reference the last_block assignment and the docker
exec/sourcehub-df-test invocation to locate where to implement these changes.
- Around line 1-2: Replace the non-portable shebang in tests/docker/test.sh
(currently "#!/usr/bin/bash") with a portable one using the env lookup (e.g.,
"#!/usr/bin/env bash") so the script finds bash across different systems; update
the first line accordingly and ensure the rest of the script continues to use
bash-compatible features (the following "set -e" and other bash usage need no
change).
🪄 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: 7e4d4cd9-e655-4e92-a2d3-0992422d45af
📒 Files selected for processing (7)
Makefiledocker/entrypoint.shtests/docker/genesis.jsontests/docker/mnemonictests/docker/node_key.jsontests/docker/priv_validator_key.jsontests/docker/test.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (8)
tests/docker/mnemonic (1)
1-1: Test-only secret fixture - acceptable for testing purposes.This mnemonic is committed as a test fixture, which is appropriate for reproducible Docker integration tests. Ensure this key is never used for any real funds or production environments.
Makefile (1)
52-54: LGTM!The
dockertarget is unchanged; only a trailing newline was added for POSIX compliance.tests/docker/node_key.json (1)
1-1: LGTM!Test fixture for node P2P identity. Appropriately placed in
tests/docker/directory to indicate test-only usage.tests/docker/priv_validator_key.json (1)
1-1: LGTM!The validator public key correctly matches the
MsgCreateValidatorpubkey intests/docker/genesis.json, ensuring the test node can produce blocks.docker/entrypoint.sh (2)
9-9: Improved initialization check using genesis.json presence.Checking for
genesis.jsoninstead of a marker file is more robust since it validates the actual dependency.
22-22: Good: Removed output suppression for better debugging.Allowing init output/errors to be visible will help diagnose container startup issues.
tests/docker/genesis.json (2)
329-341: Potential denom mismatch:mint_denomis "stake" butbond_denomis "uopen".The mint module is configured to mint
staketokens (line 335), but the staking module usesuopenas the bond denomination (line 362). This mismatch means newly minted inflation rewards won't be in the staking denomination, which could cause unexpected behavior in longer-running tests.If this is intentional for testing purposes, consider adding a comment in the genesis file or a README explaining the configuration. Otherwise, align the denominations:
"mint_denom": "uopen",
1-7: LGTM - Genesis structure is valid.The genesis file correctly configures the test chain with matching validator keys and initial account balances. The
chain_id"sourcehub-shinzo" clearly indicates this is a test/dev chain.
| docker run --rm \ | ||
| -e MNEMONIC_PATH=/secrets/mnemonic \ | ||
| -e CONSENSUS_KEY_PATH=/secrets/priv_validator_key.json \ | ||
| -e COMET_NODE_KEY_PATH=/secrets/node_key.json \ | ||
| -e GENESIS_PATH=/secrets/genesis.json \ | ||
| -v .:/secrets \ | ||
| --name sourcehub-df-test \ | ||
| --user 1000:1000 \ | ||
| --read-only \ | ||
| ghcr.io/sourcenetwork/sourcehub:dev & |
There was a problem hiding this comment.
Volume mount assumes specific working directory.
The -v .:/secrets mount assumes the script is executed from the tests/docker/ directory. If run from the repository root, the wrong files will be mounted.
🔧 Proposed fix using script directory
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
docker run --rm \
-e MNEMONIC_PATH=/secrets/mnemonic \
-e CONSENSUS_KEY_PATH=/secrets/priv_validator_key.json \
-e COMET_NODE_KEY_PATH=/secrets/node_key.json \
-e GENESIS_PATH=/secrets/genesis.json \
- -v .:/secrets \
+ -v "$SCRIPT_DIR":/secrets \
--name sourcehub-df-test \
--user 1000:1000 \
--read-only \
ghcr.io/sourcenetwork/sourcehub:dev &📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker run --rm \ | |
| -e MNEMONIC_PATH=/secrets/mnemonic \ | |
| -e CONSENSUS_KEY_PATH=/secrets/priv_validator_key.json \ | |
| -e COMET_NODE_KEY_PATH=/secrets/node_key.json \ | |
| -e GENESIS_PATH=/secrets/genesis.json \ | |
| -v .:/secrets \ | |
| --name sourcehub-df-test \ | |
| --user 1000:1000 \ | |
| --read-only \ | |
| ghcr.io/sourcenetwork/sourcehub:dev & | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| docker run --rm \ | |
| -e MNEMONIC_PATH=/secrets/mnemonic \ | |
| -e CONSENSUS_KEY_PATH=/secrets/priv_validator_key.json \ | |
| -e COMET_NODE_KEY_PATH=/secrets/node_key.json \ | |
| -e GENESIS_PATH=/secrets/genesis.json \ | |
| -v "$SCRIPT_DIR":/secrets \ | |
| --name sourcehub-df-test \ | |
| --user 1000:1000 \ | |
| --read-only \ | |
| ghcr.io/sourcenetwork/sourcehub:dev & |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/docker/test.sh` around lines 4 - 13, The docker run in
tests/docker/test.sh mounts the current working directory with "-v .:/secrets",
which breaks if the script is invoked from a different directory; update the
docker run command to compute the script's directory and mount that absolute
path instead of "." (e.g., derive the script directory with dirname/realpath or
cd+pwd and use that path in place of "-v .:/secrets") so the container always
receives the intended secret files when running the docker run invocation that
includes MNEMONIC_PATH, CONSENSUS_KEY_PATH, COMET_NODE_KEY_PATH and
GENESIS_PATH.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/docker/docker_test.go (2)
89-102: Minor:t.Helper()is misplaced and error fromReadFromis ignored.
t.Helper()should be used in helper functions called by tests, not in cleanup functions. It can be removed here.- The error from
buf.ReadFrom(logs)is silently discarded. While this is cleanup code where errors have minimal impact, it would be cleaner to handle it.♻️ Suggested improvement
t.Cleanup(func() { - t.Helper() logs, err := container.Logs(ctx) if err != nil { t.Logf("could not read container logs") } else { buf := bytes.Buffer{} - // errors during cleanup don't affect anything - buf.ReadFrom(logs) + if _, err := buf.ReadFrom(logs); err != nil { + t.Logf("error reading container logs: %v", err) + } t.Logf("container logs: %v", buf.String()) logs.Close() } testcontainers.TerminateContainer(container) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/docker/docker_test.go` around lines 89 - 102, Remove the misplaced t.Helper() call from the anonymous cleanup passed to t.Cleanup and handle the error returned by buf.ReadFrom(logs) instead of discarding it: after obtaining logs via container.Logs(ctx) call buf.ReadFrom(logs), check its error and call t.Logf with a message including that error if non-nil (still treating it as non-fatal), then log the buffer contents as before and ensure logs.Close() is called; reference the anonymous cleanup function created in the t.Cleanup call, the container.Logs(ctx) call, and the buf.ReadFrom(logs) invocation when making the change.
118-136: Clarify: Backoff is linear, not exponential.The comment says "exponential backoff" but the implementation is linear:
i * 10msproduces delays of 10ms, 20ms, 30ms, etc. True exponential would be2^i * base. This is fine for a 10-second timeout test, but the comment is misleading.Also, consider capping the backoff to avoid very long waits between retries near the end of the timeout period.
♻️ Option 1: Fix the comment to match behavior
for { - // use an exponential backoff timer to adjust polling + // use a linear backoff timer to adjust polling timer := time.After(time.Duration(i) * (10 * time.Millisecond))♻️ Option 2: Implement actual exponential backoff with cap
- i := 1 + backoff := 50 * time.Millisecond + maxBackoff := 500 * time.Millisecond startTs := time.Now() for { - // use an exponential backoff timer to adjust polling - timer := time.After(time.Duration(i) * (10 * time.Millisecond)) - i++ + timer := time.After(backoff) select { case <-ctx.Done(): t.Logf("time out waiting for chain to start") return fmt.Errorf("error setting up chain: connection not ready after deadline") case <-timer: ok := probeChain(t, ctx, grpcEndpoint, cometRpcEndpoint) if ok { elapsed := time.Since(startTs) t.Logf("chain ready to receive connections: after %v", elapsed) return nil } + // exponential backoff with cap + backoff = min(backoff*2, maxBackoff) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/docker/docker_test.go` around lines 118 - 136, The comment wrongly calls the loop "exponential backoff"; update the implementation to match or vice‑versa — I suggest implementing real exponential backoff with a cap: compute delay using a base (e.g., base := 10*time.Millisecond) multiplied by 2^(i-1) (use bitshift on i), then clamp to a maxDelay (e.g., 500ms) before creating timer := time.After(delay); increment i but bound it so shifts don't overflow, and keep probeChain(t, ctx, grpcEndpoint, cometRpcEndpoint) and the ctx.Done handling the same; also update the inline comment to describe the capped exponential backoff.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 61: Update the grpc dependency declaration for google.golang.org/grpc in
go.mod from v1.76.0 to v1.79.3 (or a later patched release) to address CVE
GO-2026-4762; after changing the version string for the module entry
"google.golang.org/grpc", run the module update (go get
google.golang.org/grpc@v1.79.3 or equivalent), then run go mod tidy and the
project test/build to ensure compatibility and resolve any transitive version
changes.
In `@tests/docker/docker_test.go`:
- Line 52: Fix the typo in the test log message: update the t.Logf call that
currently prints "writting file to tmp dir: %v" to "writing file to tmp dir: %v"
(locate the t.Logf invocation in tests/docker/docker_test.go and change the
string only).
- Around line 53-54: The os.WriteFile call uses a hexadecimal literal (0x777)
for permissions; change the mode argument in the os.WriteFile(path,
[]byte(data), 0x777) call to an octal literal like 0o777 for rwxrwxrwx or,
preferably, a more appropriate permission such as 0o644 for config files (or
0o755 if the file must be executable) to avoid setting incorrect permission
bits.
- Around line 60-65: The ContainerFile literal sets FileMode using a hexadecimal
literal (0x777); change it to an octal literal (0777) so permissions are correct
— update the FileMode field in the testcontainers.ContainerFile instance
(variable f) to use 0777 and scan other ContainerFile initializations in the
same test for similar fixes to keep permission semantics intact.
---
Nitpick comments:
In `@tests/docker/docker_test.go`:
- Around line 89-102: Remove the misplaced t.Helper() call from the anonymous
cleanup passed to t.Cleanup and handle the error returned by buf.ReadFrom(logs)
instead of discarding it: after obtaining logs via container.Logs(ctx) call
buf.ReadFrom(logs), check its error and call t.Logf with a message including
that error if non-nil (still treating it as non-fatal), then log the buffer
contents as before and ensure logs.Close() is called; reference the anonymous
cleanup function created in the t.Cleanup call, the container.Logs(ctx) call,
and the buf.ReadFrom(logs) invocation when making the change.
- Around line 118-136: The comment wrongly calls the loop "exponential backoff";
update the implementation to match or vice‑versa — I suggest implementing real
exponential backoff with a cap: compute delay using a base (e.g., base :=
10*time.Millisecond) multiplied by 2^(i-1) (use bitshift on i), then clamp to a
maxDelay (e.g., 500ms) before creating timer := time.After(delay); increment i
but bound it so shifts don't overflow, and keep probeChain(t, ctx, grpcEndpoint,
cometRpcEndpoint) and the ctx.Done handling the same; also update the inline
comment to describe the capped exponential backoff.
🪄 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: 81a7fb60-477e-4dae-b9e0-00649d4939ff
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modtests/docker/docker_test.gotests/docker/files/genesis.jsontests/docker/files/mnemonictests/docker/files/node_key.jsontests/docker/files/priv_validator_key.json
✅ Files skipped from review due to trivial changes (3)
- tests/docker/files/mnemonic
- tests/docker/files/node_key.json
- tests/docker/files/priv_validator_key.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🧰 Additional context used
🪛 Betterleaks (1.1.1)
tests/docker/files/genesis.json
[high] 33-33: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 43-43: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 171-171: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 189-189: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 OSV Scanner (2.3.5)
go.mod
[CRITICAL] 61-61: google.golang.org/grpc 1.76.0: Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc
(GO-2026-4762)
[CRITICAL] 61-61: google.golang.org/grpc 1.76.0: gRPC-Go has an authorization bypass via missing leading slash in :path
[HIGH] 186-186: github.com/docker/docker 28.5.2+incompatible: Moby has an Off-by-one error in its plugin privilege validation
[HIGH] 186-186: github.com/docker/docker 28.5.2+incompatible: Moby has AuthZ plugin bypass when provided oversized request bodies
🔇 Additional comments (4)
go.mod (2)
57-64: LGTM: New test dependencies look appropriate.The addition of
testcontainers-go v0.41.0and updates togolang.org/x/toolsandgotest.tools/v3are appropriate for the new Docker integration test infrastructure.
186-186: Remove vulnerability-specific claims pending further validation.
github.com/docker/dockerv28.5.2 is used transitively viatestcontainers-gofor integration tests only. Since this dependency is limited to test infrastructure, production risk is minimal. Monitortestcontainers-goreleases for updates to the bundled Docker client version.> Likely an incorrect or invalid review comment.tests/docker/files/genesis.json (1)
1-417: LGTM: Genesis test fixture is well-structured.This genesis file provides a complete Cosmos SDK chain configuration for integration testing. The structure is correct, with properly matching balances and supply totals.
Note: The static analysis warnings about "API keys" on lines 33, 43, 171, and 189 are false positives. These are Cosmos cryptographic public keys (secp256k1 and ed25519), which are inherently public data and not secrets. Public keys are meant to be shared and do not pose a security risk.
tests/docker/docker_test.go (1)
38-112: Good test structure overall.The test correctly:
- Uses
t.TempDir()for automatic cleanup- Embeds test fixtures for reproducibility
- Properly terminates containers in cleanup
- Logs container output for debugging failed tests
- Uses the SDK client API correctly per the relevant code snippets
| golang.org/x/tools v0.38.0 | ||
| golang.org/x/tools v0.41.0 | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20251103181224-f26f9409b101 | ||
| google.golang.org/grpc v1.76.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest version of google.golang.org/grpc and which versions fix the authorization bypass vulnerability GHSA-p77j-4mvh-x3m3?
💡 Result:
The latest version of google.golang.org/grpc is v1.79.3 (released 2026-03-17). This version fixes the authorization bypass vulnerability GHSA-p77j-4mvh-x3m3. Affected versions are those below v1.79.3; upgrade to v1.79.3 or newer.
Citations:
- 1: https://github.com/grpc/grpc-go/releases
- 2: https://pkg.go.dev/google.golang.org/grpc?tab=versions
- 3: https://pkg.go.dev/google.golang.org/grpc
- 4: https://github.com/grpc/grpc-go
- 5: https://github.com/grpc/grpc-go/tags
- 6: GHSA-p77j-4mvh-x3m3
- 7: https://github.com/grpc/grpc-go/releases/tag/v1.79.3
Upgrade google.golang.org/grpc to v1.79.3 or later to fix authorization bypass vulnerability.
The current version v1.76.0 contains GHSA-p77j-4mvh-x3m3 (CVE GO-2026-4762), a critical authorization bypass vulnerability in the :path header validation. Upgrade to v1.79.3 or newer to resolve this issue.
🧰 Tools
🪛 OSV Scanner (2.3.5)
[CRITICAL] 61-61: google.golang.org/grpc 1.76.0: Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc
(GO-2026-4762)
[CRITICAL] 61-61: google.golang.org/grpc 1.76.0: gRPC-Go has an authorization bypass via missing leading slash in :path
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 61, Update the grpc dependency declaration for
google.golang.org/grpc in go.mod from v1.76.0 to v1.79.3 (or a later patched
release) to address CVE GO-2026-4762; after changing the version string for the
module entry "google.golang.org/grpc", run the module update (go get
google.golang.org/grpc@v1.79.3 or equivalent), then run go mod tidy and the
project test/build to ensure compatibility and resolve any transitive version
changes.
There was a problem hiding this comment.
I think this suggestion is valid
There was a problem hiding this comment.
@iverc, confirmed. This is an Authorization Bypass (CWE-285) resulting from Improper Input Validation of the HTTP/2 :path pseudo-header — the gRPC-Go server accepted requests where :path omitted the mandatory leading slash (e.g., Service/Method instead of /Service/Method). While the server successfully routed these requests to the correct handler, authorization interceptors evaluated the raw, non-canonical path string, causing "deny" rules defined using canonical paths to fail to match, allowing requests to bypass the policy if a fallback "allow" rule was present.
The advisory has a CVSS score of 9.1 (Critical). All versions of google.golang.org/grpc below v1.79.3 are affected, and v1.79.3 is the patched release.
Here's the fix for go.mod:
- google.golang.org/grpc v1.76.0
+ google.golang.org/grpc v1.79.3Then run:
go get google.golang.org/grpc@v1.79.3
go mod tidy| } | ||
| for fileName, data := range fileMap { | ||
| path := path.Join(tmpDir, fileName) | ||
| t.Logf("writting file to tmp dir: %v", path) |
There was a problem hiding this comment.
Typo: "writting" should be "writing".
✏️ Fix typo
- t.Logf("writting file to tmp dir: %v", path)
+ t.Logf("writing file to tmp dir: %v", path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| t.Logf("writting file to tmp dir: %v", path) | |
| t.Logf("writing file to tmp dir: %v", path) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/docker/docker_test.go` at line 52, Fix the typo in the test log
message: update the t.Logf call that currently prints "writting file to tmp dir:
%v" to "writing file to tmp dir: %v" (locate the t.Logf invocation in
tests/docker/docker_test.go and change the string only).
| err := os.WriteFile(path, []byte(data), 0x777) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
Bug: File permission 0x777 should be 0o777 (octal notation).
The value 0x777 is hexadecimal (1911 in decimal), which sets unusual permission bits including setuid/setgid. The intended permission rwxrwxrwx requires octal 0o777 (or the older 0777 notation).
This could cause issues with file permissions in the container.
🐛 Proposed fix
- err := os.WriteFile(path, []byte(data), 0x777)
+ err := os.WriteFile(path, []byte(data), 0o644)Note: 0o644 (rw-r--r--) is typically sufficient for config files. If executable permissions are truly needed, use 0o755.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| err := os.WriteFile(path, []byte(data), 0x777) | |
| require.NoError(t, err) | |
| err := os.WriteFile(path, []byte(data), 0o644) | |
| require.NoError(t, err) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/docker/docker_test.go` around lines 53 - 54, The os.WriteFile call uses
a hexadecimal literal (0x777) for permissions; change the mode argument in the
os.WriteFile(path, []byte(data), 0x777) call to an octal literal like 0o777 for
rwxrwxrwx or, preferably, a more appropriate permission such as 0o644 for config
files (or 0o755 if the file must be executable) to avoid setting incorrect
permission bits.
| f := testcontainers.ContainerFile{ | ||
| HostFilePath: path.Join(tmpDir, fileName), | ||
| ContainerFilePath: path.Join(containerBaseDir, fileName), | ||
| FileMode: 0x777, | ||
| } | ||
| containerFiles = append(containerFiles, f) |
There was a problem hiding this comment.
Same issue: FileMode: 0x777 should use octal notation.
Consistent with the previous comment, this should use octal permissions.
🐛 Proposed fix
f := testcontainers.ContainerFile{
HostFilePath: path.Join(tmpDir, fileName),
ContainerFilePath: path.Join(containerBaseDir, fileName),
- FileMode: 0x777,
+ FileMode: 0o644,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| f := testcontainers.ContainerFile{ | |
| HostFilePath: path.Join(tmpDir, fileName), | |
| ContainerFilePath: path.Join(containerBaseDir, fileName), | |
| FileMode: 0x777, | |
| } | |
| containerFiles = append(containerFiles, f) | |
| f := testcontainers.ContainerFile{ | |
| HostFilePath: path.Join(tmpDir, fileName), | |
| ContainerFilePath: path.Join(containerBaseDir, fileName), | |
| FileMode: 0o644, | |
| } | |
| containerFiles = append(containerFiles, f) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/docker/docker_test.go` around lines 60 - 65, The ContainerFile literal
sets FileMode using a hexadecimal literal (0x777); change it to an octal literal
(0777) so permissions are correct — update the FileMode field in the
testcontainers.ContainerFile instance (variable f) to use 0777 and scan other
ContainerFile initializations in the same test for similar fixes to keep
permission semantics intact.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #96 +/- ##
==========================================
+ Coverage 47.84% 47.87% +0.03%
==========================================
Files 276 276
Lines 16195 16195
==========================================
+ Hits 7749 7754 +5
+ Misses 7642 7638 -4
+ Partials 804 803 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
iverc
left a comment
iverc
left a comment
There was a problem hiding this comment.
please check coderabbit suggestion regarding typos and perms, lgtm otherwise
3 participants
No description provided.