Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughBumps Go toolchain to 1.25 across CI, Dockerfile, and go.mod; updates many dependencies (notably OpenTelemetry, gRPC, protobuf). Adds env-file support to startup scripts, URL safety checks before HPSM fetches, Makefile lint targets, a golangci-lint config rewrite, and several lint-suppression and cleanup changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (2)
Dockerfile (1)
19-27: Consider running as a non-root user for improved security posture.The container currently runs as root. While this may be acceptable depending on your deployment model, running as a non-root user reduces the attack surface if the container is compromised.
🛡️ Optional: Add non-root user
FROM debian:bookworm-slim AS production +RUN useradd --system --no-create-home scanoss + WORKDIR /app COPY --from=build /app/scanoss-go-api /app/scanoss-go-api +USER scanoss + EXPOSE 5443 ENTRYPOINT ["./scanoss-go-api"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 19 - 27, The image runs the binary as root; create a non-root user and ensure the app files are owned by that user, then switch to it before ENTRYPOINT: add steps in the Dockerfile to create a dedicated user/group (e.g., appuser), chown /app/scanoss-go-api to that user, set a safe HOME if needed, and add a USER appuser directive so ENTRYPOINT ["./scanoss-go-api"] runs without root privileges; ensure any required ports or capabilities are handled by the runtime rather than granting extra permissions in the image.Makefile (1)
100-104: Echo message may be misleading forbuild_localtarget.The echo says "Building AMD binary" but
build_localdoesn't specifyGOOS/GOARCH, so it builds for the host machine's architecture (which could be ARM on macOS, for example). Consider updating the message to reflect the actual behavior.📝 Suggested improvement
build_local: version ## Build a Local binary - `@echo` "Building AMD binary $(VERSION)..." + `@echo` "Building local binary $(VERSION)..." go generate ./pkg/cmd/server.go CGO_ENABLED=0 go build -ldflags="-w -s" -o ./target/scanoss-go-api-local ./cmd/server🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 100 - 104, The echo in the build_local Makefile target ("Building AMD binary $(VERSION)...") is misleading because build_local doesn't set GOOS/GOARCH; either update the message in the build_local recipe to reflect it builds a local/host binary (e.g., "Building local binary $(VERSION) for host architecture") or explicitly set the intended GOARCH/GOOS in the build_local target (e.g., export GOARCH=amd64/GOOS=linux) so the message matches behavior; modify the build_local target and its echo line accordingly (refer to the build_local target and the echo that prints "Building AMD binary $(VERSION)...").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 197: The link reference definition currently duplicates `[1.6.2]` for the
new release; update the reference label to `[1.6.3]` in the CHANGELOG.md link
definition so it matches the new release section — replace the existing
`[1.6.2]: https://github.com/scanoss/api.go/compare/v1.6.2...v1.6.3` entry with
`[1.6.3]: https://github.com/scanoss/api.go/compare/v1.6.2...v1.6.3`.
In `@scripts/env-setup.sh`:
- Around line 146-150: Replace the typo `eco` with `echo` and fix the malformed
nested quoting in the error message: ensure the `$ENV_PATH` and `$RUNTIME_USER`
variables are correctly quoted (for example use double quotes around the whole
string and escape inner quotes or simply include the variables unquoted inside
the outer quotes) in the failure echo so the path is preserved and no
word-splitting occurs; update the chmod error branch that references ENV_PATH,
RUNTIME_USER and chmod 600 accordingly.
In `@scripts/scanoss-go-api.sh`:
- Around line 25-29: The guard that adds the env file currently only checks
existence of ENV_FILE; change it to verify the file is readable (e.g., test -r
"$ENV_FILE") and if it exists but is not readable, print a clear error referring
to ENV_FILE and exit non‑zero so the script fails fast; if it is readable,
append the --env-config "$ENV_FILE" to CMD_ARGS as before.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 19-27: The image runs the binary as root; create a non-root user
and ensure the app files are owned by that user, then switch to it before
ENTRYPOINT: add steps in the Dockerfile to create a dedicated user/group (e.g.,
appuser), chown /app/scanoss-go-api to that user, set a safe HOME if needed, and
add a USER appuser directive so ENTRYPOINT ["./scanoss-go-api"] runs without
root privileges; ensure any required ports or capabilities are handled by the
runtime rather than granting extra permissions in the image.
In `@Makefile`:
- Around line 100-104: The echo in the build_local Makefile target ("Building
AMD binary $(VERSION)...") is misleading because build_local doesn't set
GOOS/GOARCH; either update the message in the build_local recipe to reflect it
builds a local/host binary (e.g., "Building local binary $(VERSION) for host
architecture") or explicitly set the intended GOARCH/GOOS in the build_local
target (e.g., export GOARCH=amd64/GOOS=linux) so the message matches behavior;
modify the build_local target and its echo line accordingly (refer to the
build_local target and the echo that prints "Building AMD binary
$(VERSION)...").
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 839f1dea-246e-4b92-805e-32e3b37a5284
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
.github/workflows/go-ci.yml.github/workflows/golangci-lint.yml.github/workflows/release.ymlCHANGELOG.mdDockerfileMakefilego.modscripts/env-setup.shscripts/scanoss-go-api.sh
scripts/env-setup.sh
Outdated
Comment on lines
+146
to
+150
| eco "Make $ENV_CONF readable/writable only to $RUNTIME_USER ..." | ||
| if ! chmod 600 "$ENV_PATH" ; then | ||
| echo "chmod of "$ENV_PATH" to $RUNTIME_USER failed" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fix typo and quoting issues in permission handling block.
- Line 146:
ecoshould beecho- this will cause a "command not found" error. - Line 148: The nested quotes around
$ENV_PATHwill cause the variable to be unquoted in the error message.
🐛 Proposed fix
if [ -f "$ENV_PATH" ] ; then
- eco "Make $ENV_CONF readable/writable only to $RUNTIME_USER ..."
+ echo "Make $ENV_CONF readable/writable only to $RUNTIME_USER ..."
if ! chmod 600 "$ENV_PATH" ; then
- echo "chmod of "$ENV_PATH" to $RUNTIME_USER failed"
+ echo "chmod of $ENV_PATH to $RUNTIME_USER failed"
exit 1
fi
fi📝 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.
Suggested change
| eco "Make $ENV_CONF readable/writable only to $RUNTIME_USER ..." | |
| if ! chmod 600 "$ENV_PATH" ; then | |
| echo "chmod of "$ENV_PATH" to $RUNTIME_USER failed" | |
| exit 1 | |
| fi | |
| echo "Make $ENV_CONF readable/writable only to $RUNTIME_USER ..." | |
| if ! chmod 600 "$ENV_PATH" ; then | |
| echo "chmod of $ENV_PATH to $RUNTIME_USER failed" | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 148-148: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/env-setup.sh` around lines 146 - 150, Replace the typo `eco` with
`echo` and fix the malformed nested quoting in the error message: ensure the
`$ENV_PATH` and `$RUNTIME_USER` variables are correctly quoted (for example use
double quotes around the whole string and escape inner quotes or simply include
the variables unquoted inside the outer quotes) in the failure echo so the path
is preserved and no word-splitting occurs; update the chmod error branch that
references ENV_PATH, RUNTIME_USER and chmod 600 accordingly.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
pkg/service/kb_details.go (1)
143-143: Minor: Fix pre-existing typo "eninge" → "engine".Since this line is being touched, consider fixing the typo in the error message.
📝 Proposed fix
- zs.Warnf("Failed to parse KB version from eninge result: %v - %v", data, err2) + zs.Warnf("Failed to parse KB version from engine result: %v - %v", data, err2)Note: The same typo appears on lines 116 and 127 which could also be fixed for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/service/kb_details.go` at line 143, The error message in the zs.Warnf call contains a typo "eninge" — update the message string in the zs.Warnf invocation that currently reads "Failed to parse KB version from eninge result: %v - %v" to "Failed to parse KB version from engine result: %v - %v", and also search for and correct the same misspelling in the other similar zs.Warnf/log messages in the same file to keep them consistent.pkg/cmd/server.go (2)
218-219: Unnecessary//nolint:gosecannotation.The
gosecG107 rule flags the URL at the point where it's passed tohttp.NewRequest, not atclient.Do. This annotation on line 218 has no effect and can be removed.🧹 Proposed fix
// Perform the request with a 10-second timeout client := &http.Client{Timeout: 10 * time.Second} - //nolint:gosec resp, err := client.Do(req)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/server.go` around lines 218 - 219, Remove the unnecessary gosec suppression placed on the client.Do call: the G107 warning applies when creating the request URL (http.NewRequest) not when executing it, so delete the "//nolint:gosec" comment that precedes "resp, err := client.Do(req)"; ensure any remaining gosec concerns are addressed at the http.NewRequest site (where "req" is created) rather than on the "client.Do" line.
242-255: Consider makingisSafeURLmore robust and configurable.A few considerations for this validation:
- Case-insensitivity: Hostnames are case-insensitive per RFC; consider using
strings.EqualFoldor lowercasing before comparison.- Localhost variants:
127.0.0.1and::1are not covered.- Configurability: Hard-coded allowed hosts limit flexibility for different deployment environments.
♻️ Example improvement for case-insensitive matching and localhost variants
func isSafeURL(rawURL string) bool { - allowed := []string{"api.scanoss.com", "osskb.org", "localhost"} + allowed := []string{"api.scanoss.com", "osskb.org", "localhost", "127.0.0.1", "::1"} u, err := url.Parse(rawURL) if err != nil { return false } + hostname := strings.ToLower(u.Hostname()) for _, host := range allowed { - if u.Hostname() == host { + if hostname == host { return true } } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/server.go` around lines 242 - 255, The isSafeURL function should be made case-insensitive, accept localhost IP variants, and read allowed hosts from configuration instead of being hard-coded: update isSafeURL to canonicalize the parsed host (use u.Hostname() and strings.ToLower or strings.EqualFold for comparisons), treat "localhost", "127.0.0.1" and "::1" as safe (or include them in the configurable allowlist), and replace the inline allowed slice with a configurable list (e.g., injected via config/env or a package-level variable) so different deployments can extend allowed hosts; ensure you still parse the URL with url.Parse and compare only the hostname portion (handle potential host:port via u.Hostname()) in the new implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cmd/server.go`:
- Around line 203-205: The isSafeURL check currently only logs a warning and
allows the request to proceed; replace that behavior by returning an error (or
otherwise aborting) when isSafeURL(hpsmURL) is false so unsafe URLs are never
requested. Locate the code that uses hpsmURL (the condition calling isSafeURL
and the subsequent HTTP request logic) and change the branch to return an error
(or write an appropriate HTTP error response) instead of just zlog.S.Warnf,
e.g., propagate an error from the surrounding function or short-circuit before
the HTTP client is invoked; keep the check using isSafeURL and ensure callers
handle the returned error so SSRF is prevented.
In `@pkg/protocol/rest/server.go`:
- Around line 71-74: The warning on TestEngine failure omits the actual error;
update the apiService.TestEngine() error handling to include the error in the
log message by passing the returned error to zlog.S.Warnf (or zlog.S.Warnw) so
the log shows both the descriptive message and err from TestEngine, referencing
TestEngine, apiService, zlog.S.Warnf (or Warnw) and config.Scanning.ScanBinary
to form a single informative warning that includes the error details.
- Around line 140-142: The shutdown branch currently drops the underlying error
(err2); preserve the cause by returning or wrapping err2 instead of discarding
it. Locate the srv.Shutdown(ctx) error handling (the err2 variable) and change
the return to include the original error (e.g., wrap with fmt.Errorf("issue
encountered while shutting down service: %w", err2) or return err2 directly) so
callers/tests can inspect the shutdown failure.
In `@pkg/service/scanning_service.go`:
- Around line 109-112: The error branch after calling
writeSbomFile(scanConfig.sbomFile, zs) can leak an open temp file; update the
err2 != nil path in scanning_service.go to check if tempFile is non-nil and then
close and remove it (close the descriptor and delete the temp file, e.g. call
tempFile.Close() and os.Remove(tempFile.Name())) before returning the HTTP
error; make sure to handle/ignore close/remove errors so the original error path
still returns the same response.
In `@pkg/service/utils_service.go`:
- Around line 289-290: The nolint directive "//nolint:gosec" above the call to
os.Remove(f.Name()) is unused and causes nolintlint to fail; remove that
directive so the file contains only the os.Remove(f.Name()) call (or
relocate/add a matching gosec justification if a true gosec issue exists) — look
for the os.Remove call in utils_service.go (the err := os.Remove(f.Name())
statement) and delete the "//nolint:gosec" comment above it.
---
Nitpick comments:
In `@pkg/cmd/server.go`:
- Around line 218-219: Remove the unnecessary gosec suppression placed on the
client.Do call: the G107 warning applies when creating the request URL
(http.NewRequest) not when executing it, so delete the "//nolint:gosec" comment
that precedes "resp, err := client.Do(req)"; ensure any remaining gosec concerns
are addressed at the http.NewRequest site (where "req" is created) rather than
on the "client.Do" line.
- Around line 242-255: The isSafeURL function should be made case-insensitive,
accept localhost IP variants, and read allowed hosts from configuration instead
of being hard-coded: update isSafeURL to canonicalize the parsed host (use
u.Hostname() and strings.ToLower or strings.EqualFold for comparisons), treat
"localhost", "127.0.0.1" and "::1" as safe (or include them in the configurable
allowlist), and replace the inline allowed slice with a configurable list (e.g.,
injected via config/env or a package-level variable) so different deployments
can extend allowed hosts; ensure you still parse the URL with url.Parse and
compare only the hostname portion (handle potential host:port via u.Hostname())
in the new implementation.
In `@pkg/service/kb_details.go`:
- Line 143: The error message in the zs.Warnf call contains a typo "eninge" —
update the message string in the zs.Warnf invocation that currently reads
"Failed to parse KB version from eninge result: %v - %v" to "Failed to parse KB
version from engine result: %v - %v", and also search for and correct the same
misspelling in the other similar zs.Warnf/log messages in the same file to keep
them consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f06500e-d2c2-4d6c-bba7-74952cc94beb
📒 Files selected for processing (13)
.github/workflows/golangci-lint.yml.golangci.ymlMakefilepkg/cmd/server.gopkg/config/server_config.gopkg/protocol/rest/server.gopkg/service/attribution_service.gopkg/service/filecontents_service.gopkg/service/kb_details.gopkg/service/licensedetails_service.gopkg/service/scanning_service.gopkg/service/utils_service.gopkg/service/utils_service_test.go
✅ Files skipped from review due to trivial changes (3)
- pkg/service/filecontents_service.go
- pkg/service/utils_service_test.go
- pkg/config/server_config.go
🚧 Files skipped from review as they are similar to previous changes (2)
- Makefile
- .github/workflows/golangci-lint.yml
Comment on lines
+203
to
+205
| if !isSafeURL(hpsmURL) { | ||
| zlog.S.Warnf("Disallowed URL for HPSM: %s", hpsmURL) | ||
| } |
There was a problem hiding this comment.
URL safety check is ineffective – unsafe URLs are still requested.
The isSafeURL check logs a warning but does not prevent the HTTP request from proceeding. If the goal is to mitigate SSRF risk (which the //nolint:gosec annotations suggest), the function should return an error for disallowed URLs rather than just logging.
🛡️ Proposed fix to enforce the safety check
if !isSafeURL(hpsmURL) {
- zlog.S.Warnf("Disallowed URL for HPSM: %s", hpsmURL)
+ return fmt.Errorf("disallowed URL for HPSM: %s", hpsmURL)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/cmd/server.go` around lines 203 - 205, The isSafeURL check currently only
logs a warning and allows the request to proceed; replace that behavior by
returning an error (or otherwise aborting) when isSafeURL(hpsmURL) is false so
unsafe URLs are never requested. Locate the code that uses hpsmURL (the
condition calling isSafeURL and the subsequent HTTP request logic) and change
the branch to return an error (or write an appropriate HTTP error response)
instead of just zlog.S.Warnf, e.g., propagate an error from the surrounding
function or short-circuit before the HTTP client is invoked; keep the check
using isSafeURL and ensure callers handle the returned error so SSRF is
prevented.
There was a problem hiding this comment.
@agustingroh, great question — it depends on the intended threat model:
Case for failing hard (returning an error):
If isSafeURL is meant as an SSRF guard, a warning-only path is effectively decorative — the HTTP request still goes out to the disallowed host. Given that the //nolint:gosec annotations in the surrounding code show awareness of SSRF risk, a hard failure would be the consistent and correct behavior.
Case for warning only (current behavior):
The URL here comes from operator-controlled configuration (SCANOSS_FILE_CONTENTS_URL), not from end-user input, so the SSRF surface is already reduced. More importantly, the allow-list is very narrow (api.scanoss.com, osskb.org, localhost). If any legitimate on-premise or private HPSM deployment uses a custom host, a hard failure would break those deployments silently.
Recommendation:
If HPSM is only ever expected to point to the three hardcoded hosts, fail hard — the check is meaningless otherwise. If operators can legitimately configure a private HPSM server, the allow-list approach may need to be rethought (e.g., configurable trusted hosts), and a warning is the safer default for now.
A middle ground could also be to fail hard and make the allow-list configurable via config, so private deployments aren't broken.
Comment on lines
+71
to
74
| if err2 := apiService.TestEngine(); err2 != nil { | ||
| zlog.S.Warnf("Scanning engine test failed. Scan requests are likely to fail.") | ||
| zlog.S.Warnf("Please make sure that %v is accessible", config.Scanning.ScanBinary) | ||
| } |
There was a problem hiding this comment.
Include the TestEngine failure in the warning.
This branch warns that startup is degraded, but it drops the actual error, which makes operator triage much harder.
Suggested change
if err2 := apiService.TestEngine(); err2 != nil {
- zlog.S.Warnf("Scanning engine test failed. Scan requests are likely to fail.")
+ zlog.S.Warnf("Scanning engine test failed: %v. Scan requests are likely to fail.", err2)
zlog.S.Warnf("Please make sure that %v is accessible", config.Scanning.ScanBinary)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocol/rest/server.go` around lines 71 - 74, The warning on TestEngine
failure omits the actual error; update the apiService.TestEngine() error
handling to include the error in the log message by passing the returned error
to zlog.S.Warnf (or zlog.S.Warnw) so the log shows both the descriptive message
and err from TestEngine, referencing TestEngine, apiService, zlog.S.Warnf (or
Warnw) and config.Scanning.ScanBinary to form a single informative warning that
includes the error details.
Comment on lines
+140
to
142
| if err2 := srv.Shutdown(ctx); err2 != nil { | ||
| zlog.S.Warnf("error shutting down server %s", err2) | ||
| return fmt.Errorf("issue encountered while shutting down service") |
There was a problem hiding this comment.
Preserve the shutdown cause when returning the error.
The current return value drops err2, so callers and tests lose the reason shutdown failed.
Suggested change
if err2 := srv.Shutdown(ctx); err2 != nil {
zlog.S.Warnf("error shutting down server %s", err2)
- return fmt.Errorf("issue encountered while shutting down service")
+ return fmt.Errorf("issue encountered while shutting down service: %w", err2)
}📝 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.
Suggested change
| if err2 := srv.Shutdown(ctx); err2 != nil { | |
| zlog.S.Warnf("error shutting down server %s", err2) | |
| return fmt.Errorf("issue encountered while shutting down service") | |
| if err2 := srv.Shutdown(ctx); err2 != nil { | |
| zlog.S.Warnf("error shutting down server %s", err2) | |
| return fmt.Errorf("issue encountered while shutting down service: %w", err2) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocol/rest/server.go` around lines 140 - 142, The shutdown branch
currently drops the underlying error (err2); preserve the cause by returning or
wrapping err2 instead of discarding it. Locate the srv.Shutdown(ctx) error
handling (the err2 variable) and change the return to include the original error
(e.g., wrap with fmt.Errorf("issue encountered while shutting down service: %w",
err2) or return err2 directly) so callers/tests can inspect the shutdown
failure.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/env-setup.sh (1)
145-151:⚠️ Potential issue | 🟡 MinorFix the quoting issue in the error message on line 148.
The nested quotes around
$ENV_PATHbreak the string - the inner quotes actually unquote the variable. This will cause the error message to display incorrectly.,
🐛 Proposed fix
if ! chmod 600 "$ENV_PATH" ; then - echo "chmod of "$ENV_PATH" to $RUNTIME_USER failed" + echo "chmod of $ENV_PATH to $RUNTIME_USER failed" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/env-setup.sh` around lines 145 - 151, The error echo uses nested double quotes that break the string around $ENV_PATH; update the echo to properly quote or escape the variable so the path is printed correctly — e.g. change the line referencing ENV_PATH and RUNTIME_USER to use escaped quotes around $ENV_PATH (echo "chmod of \"$ENV_PATH\" to $RUNTIME_USER failed") or otherwise concatenate safely so $ENV_PATH is not unquoted; adjust the echo that mentions ENV_PATH and RUNTIME_USER accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/env-setup.sh`:
- Around line 145-151: The error echo uses nested double quotes that break the
string around $ENV_PATH; update the echo to properly quote or escape the
variable so the path is printed correctly — e.g. change the line referencing
ENV_PATH and RUNTIME_USER to use escaped quotes around $ENV_PATH (echo "chmod of
\"$ENV_PATH\" to $RUNTIME_USER failed") or otherwise concatenate safely so
$ENV_PATH is not unquoted; adjust the echo that mentions ENV_PATH and
RUNTIME_USER accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9830279-9e2e-4658-8e28-96bb802835a3
📒 Files selected for processing (5)
CHANGELOG.mdMakefilepkg/service/utils_service.goscripts/env-setup.shscripts/scanoss-go-api.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/scanoss-go-api.sh
- pkg/service/utils_service.go
- Makefile
- CHANGELOG.md
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
agustingroh
approved these changes
Mar 10, 2026
Comment on lines
+203
to
+205
| if !isSafeURL(hpsmURL) { | ||
| zlog.S.Warnf("Disallowed URL for HPSM: %s", hpsmURL) | ||
| } |
| data, err := json.Marshal(key) // convert the given interface to JSON | ||
| if err != nil { | ||
| zs.Warnf("Failed to convert KB version map to json: %v - %v", key, err) | ||
| data, err2 := json.Marshal(key) // convert the given interface to JSON |
There was a problem hiding this comment.
could we add a TODO comment to enhance error names, please?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores