Networks#39
Merged
Merged
Conversation
- Added a new **Networks** screen in the sidebar for listing and inspecting networks, including details such as driver, scope, gateway, and options.
- Implemented backend API endpoints for network management: `GET /v1/networks`, `GET /v1/networks/{name}`, and `DELETE /v1/networks/{name}`.
- Enhanced configuration to support HTTP/HTTPS proxy settings, persisted in the config and applied to containerd in the VM.
- Updated changelog to reflect the addition of network management features and improvements.
- Introduced validation for proxy settings in the API, ensuring proper handling of HTTP/HTTPS configurations.
- Bumped version to 0.6.0 in both backend and UI configurations. - Added a new section in CHANGELOG for version 0.6.0, highlighting redesigned proxy settings with improved layout and usability. - Updated CHANGELOG entries to reflect user-facing changes and improvements in the application.
Closed
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR adds Docker/nerdctl network management (list/inspect/remove) across backend runtime implementations (Lima, Native, Mock), exposes new ChangesNetworks and proxy feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UI as NetworksScreen
participant ApiClient
participant Server as Server (API)
participant Runtime
UI->>ApiClient: fetchNetworks()
ApiClient->>Server: GET /v1/networks
Server->>Runtime: ListNetworks(ctx)
Runtime-->>Server: []Network
Server-->>ApiClient: 200 OK JSON
ApiClient-->>UI: List<NetworkItem>
UI->>ApiClient: removeNetwork(name)
ApiClient->>Server: DELETE /v1/networks/{name}
Server->>Runtime: RemoveNetwork(ctx, name)
Runtime-->>Server: ok / error
Server-->>ApiClient: 200 OK {"status":"ok"}
ApiClient-->>UI: void
sequenceDiagram
participant SettingsUI as SettingsScreen
participant ApiClient
participant Server as Server (API)
participant Runtime
SettingsUI->>ApiClient: updateConfig(httpProxy, httpsProxy, noProxy)
ApiClient->>Server: PUT /v1/config
Server->>Server: validateProxyUpdate(req)
Server->>Server: save trimmed proxy config
Server->>Runtime: ApplyProxy(ctx, ProxyConfig)
Runtime-->>Server: ok / warning logged
Server-->>ApiClient: 200 OK config response
ApiClient-->>SettingsUI: updated Config
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/runtime/lima.go (1)
64-99: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCancel the watcher when the runtime stops
Start()launcheswatchPortProxieson every successful start, andStop()never cancels it. Because the loop only exits onctx.Done(), a stop/start cycle can leave old pollers running until that context ends; keep a dedicated cancel/guard for the watcher and close it inStop().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/lima.go` around lines 64 - 99, The Lima runtime watcher is not being stopped, so repeated Start/Stop cycles can leave old watchPortProxies loops running. Add a dedicated watcher context/cancel or similar guard in the Lima type and start watchPortProxies with that lifecycle-managed context from Start; then make Stop() cancel it and clear the running state so only one watcher exists per runtime instance.
🧹 Nitpick comments (5)
backend/internal/api/handlers.go (1)
264-292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant double
SplitHostPortcall.
validateNoProxyEntrycallsnet.SplitHostPort(host)once in theifcondition and again inside the block to get the actual host value. This works but re-parses the same string twice unnecessarily.♻️ Proposed simplification
- if _, _, err := net.SplitHostPort(host); err == nil { - host, _, err = net.SplitHostPort(host) - if err == nil { - if net.ParseIP(host) != nil { - return nil - } - } - } + if splitHost, _, err := net.SplitHostPort(host); err == nil { + host = splitHost + if net.ParseIP(host) != nil { + return nil + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/api/handlers.go` around lines 264 - 292, The validateNoProxyEntry function in handlers.go is redundantly calling net.SplitHostPort twice on the same host value. Refactor that block so the host and port are extracted once, then reuse the parsed host for the net.ParseIP and isValidDomain checks; keep the existing validation behavior and error messages unchanged.backend/internal/api/networks.go (1)
16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated error-handling block could be a small helper.
The
writeRuntimeError→writeError(..., err.Error())pattern is duplicated three times in this file. A small helper (e.g.,s.writeRuntimeOrInternalError(w, err)) would reduce duplication.Also applies to: 47-55, 59-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/api/networks.go` around lines 16 - 24, The same runtime-error fallback pattern is duplicated in this file, so extract it into a small helper on the handler type, such as s.writeRuntimeOrInternalError(w, err), and use it in the ListNetworks flow and the other repeated runtime calls. Keep the helper’s behavior identical to the current writeRuntimeError followed by writeError(..., err.Error()) fallback so the existing methods remain unchanged while removing duplication.backend/internal/buildhistory/inspect.go (1)
103-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer stdlib
path.Dirover manual string splitting.
stripLastComponentreimplements whatpath.Dir(Unix-style paths, matching container/Compose paths) already provides.♻️ Proposed simplification
-func stripLastComponent(path string) string { - idx := strings.LastIndex(path, "/") - if idx < 0 { - return path - } - return path[:idx] -} +func stripLastComponent(p string) string { + return path.Dir(p) +}(Requires importing
"path", distinct from"path/filepath"to keep the Unix-style separator semantics used by container paths.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/buildhistory/inspect.go` around lines 103 - 109, The stripLastComponent helper is manually reimplementing directory extraction; replace the string-based slash handling with the stdlib path.Dir function in stripLastComponent. Add/import path (not path/filepath) so Unix-style container path semantics are preserved, and keep the function behavior aligned with the current callers in inspect.go.backend/internal/runtime/network.go (1)
249-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPer-network inspect failures are silently dropped without any diagnostic trail.
networkInspectMetadataBatchcontinues on everyinspectNetworkMetadataerror and always returns a nil error, so callers (and the deadif err != nilbranch inlistNetworksat Line 101-104) can never learn which networks failed to enrich or why. As per coding guidelines,**/*.gocode should handle each specific error case individually so failures are identifiable rather than swallowed by a generic catch-all.Consider collecting failed names and surfacing them (e.g., as a partial error or log) instead of silently continuing:
func networkInspectMetadataBatch(ctx context.Context, run commandRunner, names []string) (map[string]enrichedNetworkInspectRow, error) { rows := make(map[string]enrichedNetworkInspectRow, len(names)) + var failed []string for _, name := range names { if isPseudoNetwork(name) { continue } row, err := inspectNetworkMetadata(ctx, run, name) if err != nil { - continue + failed = append(failed, name) + continue } rows[name] = row } - - return rows, nil + if len(failed) > 0 { + return rows, fmt.Errorf("failed to inspect networks: %s", strings.Join(failed, ", ")) + } + return rows, nil }As per coding guidelines, "Do not use generic catch-alls; handle each specific error case individually so logs, UI, and API responses identify exactly what failed."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/network.go` around lines 249 - 265, networkInspectMetadataBatch currently swallows every inspectNetworkMetadata failure and returns nil, which hides per-network enrichment problems. Update networkInspectMetadataBatch to track failed network names and surface the specific inspectNetworkMetadata error for each one, either by returning a partial/aggregated error or logging the failure with the network name. Then adjust listNetworks to handle the returned error path meaningfully instead of relying on the dead err check, using the existing inspectNetworkMetadata and networkInspectMetadataBatch symbols to keep failures identifiable.Source: Coding guidelines
backend/internal/runtime/mock.go (1)
662-675: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNetwork mocking reuses
ContainersErr/ContainerErrinstead of dedicated fields.
ListNetworkstriggers onm.ContainersErrandRemoveNetworkonm.ContainerErr— both named for container-specific test scenarios. A test that setsContainersErrto simulate a container-listing failure will unexpectedly also breakListNetworks, and vice versa. DedicatedNetworksErr/NetworkErrfields would keep test scenarios isolated.Also applies to: 707-724
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/mock.go` around lines 662 - 675, `ListNetworks` and `RemoveNetwork` are incorrectly sharing container-specific error fields, so network test cases can interfere with container scenarios. Update the `Mock` runtime to use dedicated network error fields (for example `NetworksErr` and `NetworkErr`) in the `ListNetworks` and `RemoveNetwork` methods, and wire those new fields through the mock struct and any related setup so `ContainersErr`/`ContainerErr` remain container-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/api/handlers.go`:
- Around line 140-147: The `updateConfig` flow in `handlers.go` can exceed the
default `ApiClient.updateConfig().timeout(timeout)` because `ApplyProxy` is
allowed to run for up to 2 minutes, causing the UI request to fail before the
backend finishes. Update the `updateConfig` path to either use a longer client
timeout for proxy changes or return the save response before
`s.runtime.ApplyProxy` completes, and keep the fix centered on the
`proxyChanged`, `ApplyProxy`, and `ApiClient.updateConfig()` flow.
In `@backend/internal/api/networks.go`:
- Around line 46-66: handleNetworkAction has the same missing-timeout problem in
its GET and DELETE paths: both InspectNetwork and RemoveNetwork are called with
r.Context() directly. Update handleNetworkAction to create and use a context
with an explicit timeout or cancellation before calling s.runtime.InspectNetwork
and s.runtime.RemoveNetwork, matching the pattern used for other HTTP handlers
in this file.
- Around line 8-30: handleNetworks currently passes r.Context() straight into
s.runtime.ListNetworks without any explicit timeout, so a blocking VM call can
hang the request. Update handleNetworks to create a bounded context with
timeout/cancellation before calling ListNetworks, follow the same pattern used
in handleConfig for runtime operations, and ensure the derived context is always
canceled after the call. Keep the change localized to handleNetworks and the
ListNetworks invocation.
In `@backend/internal/buildhistory/inspect.go`:
- Around line 40-51: parseInspectDetail currently swallows json.Unmarshal
failures and returns a default InspectDetail, making malformed docker inspect
output indistinguishable from empty data. Change parseInspectDetail to return an
error alongside the InspectDetail, propagate the unmarshal failure instead of
falling back silently, and update the callers that depend on it (including
Inspect, enrichHistoryBuild, and resolveBuildSourcePaths) to handle and surface
that error. Also update the successful empty-input path to return an explicit
nil error so the function cleanly separates “no data” from “parse failure.”
- Around line 81-101: resolveContextFromLabels is checking compose label paths
directly with os.Stat, which fails for Lima VM paths. Update the path handling
in resolveContextFromLabels to translate com.docker.compose.project.working_dir
and com.docker.compose.project.config_files from the VM path to the host mount
root before stat’ing them, then return the translated working directory or
stripLastComponent from the translated config file path.
In `@backend/internal/runtime/lima.go`:
- Around line 92-95: The proxy application in Start() is too strict: it
currently returns the ApplyProxy error and aborts startup, unlike handleConfig
which only warns and continues. Update the Start flow around l.ApplyProxy so
proxy-apply failures are logged as warnings and do not fail the entire start
sequence, matching the non-fatal behavior used in the config-update path.
- Around line 29-36: `Lima` has unsynchronized shared state: `Status()` and the
background watcher race on `lastState`, and `ApplyProxy()` can race with
`ensureTemplate()` on `proxy`. Update the `Lima` type and its `NewLima`,
`Status`, `ApplyProxy`, and `ensureTemplate` call paths to protect these fields
with a mutex or equivalent synchronization, making sure all reads and writes of
`l.lastState` and `l.proxy` happen under the same lock.
In `@backend/internal/runtime/lima.yaml`:
- Around line 65-87: The proxy update in validateProxyUpdate/lima.yaml is still
interpolating raw proxy values into a root-run shell script, so escape or quote
the HTTP_PROXY, HTTPS_PROXY, and NO_PROXY values before embedding them in the
generated heredocs. Also make sure the generated
/etc/systemd/system/containerd.service.d/calf-proxy.conf and
/etc/profile.d/calf-proxy.sh files are written with restrictive permissions such
as 0600 when created through the sudo tee blocks.
In `@backend/internal/runtime/mock.go`:
- Around line 707-724: Mock.RemoveNetwork should match the runtime contract by
refusing removal when the runtime is not running. Add the same running-state
guard used by ListNetworks/InspectNetwork and the real lima.go/native.go
RemoveNetwork implementations, returning ErrRuntimeNotRunning before mutating
m.Networks. Keep the existing ContainerErr handling, but ensure the
runtime-state check happens first so tests can cover the “remove while stopped”
path.
- Around line 677-705: InspectNetwork currently returns a generic fmt.Errorf for
missing networks, which causes handleNetworkAction to treat the failure as an
internal error instead of a not-found response. Update the Mock.InspectNetwork
path to return a dedicated not-found sentinel for the missing network case, then
adjust handleNetworkAction to recognize that sentinel and translate it to
http.StatusNotFound while preserving ErrRuntimeNotRunning handling as-is.
In `@backend/internal/runtime/network.go`:
- Around line 81-138: The listNetworks helper returns immediately when
queryNerdctlNetworks fails, so it never applies the docker fallback used
elsewhere. Update listNetworks to continue with queryDockerNetworks and
mergeNetworksByName even when nerdctl lookup fails, using the Docker results
when nerdctl is unavailable; keep the existing metadata enrichment path in
networkInspectMetadataBatch and preserve defaultNetworkScope handling for
entries without inspect data.
In `@backend/internal/runtime/proxy.go`:
- Around line 15-62: The proxy values interpolated by applyProxyInVM are not
fully shell-safe, so malicious characters can still be executed through the bash
-lc script. Fix shellQuote to return a properly shell-escaped value that handles
spaces and shell metacharacters, and ensure applyProxyInVM uses that escaped
output consistently for every HTTPProxy/HTTPSProxy/NoProxy insertion in both the
containerd drop-in and /etc/profile.d script.
In `@ui/lib/app_shell.dart`:
- Around line 269-275: Apply should not be enabled when proxy validation errors
exist. Update the `_dirty`/Apply gating in `AppShell` so it also considers
`_httpProxyError` and `_httpsProxyError`, and keep the button disabled whenever
either validation error is set. If you choose to allow submission instead, then
`applyConfig` must surface the backend 400 instead of silently clearing
`_saving`; otherwise users can’t tell why the apply failed.
In `@ui/lib/screens/networks_screen.dart`:
- Around line 270-274: The NetworkDetailView _removeNetwork flow currently calls
removeNetwork, onRemoved, and onBack with no failure handling, so exceptions can
escape and the UI may close as if removal succeeded. Update the _removeNetwork
method in NetworkDetailView to catch removeNetwork failures, set the existing
_error state so the view can surface the problem, and only call onRemoved/onBack
after a successful removal; use the same error-handling pattern already present
in the list-view _removeNetwork for guidance.
---
Outside diff comments:
In `@backend/internal/runtime/lima.go`:
- Around line 64-99: The Lima runtime watcher is not being stopped, so repeated
Start/Stop cycles can leave old watchPortProxies loops running. Add a dedicated
watcher context/cancel or similar guard in the Lima type and start
watchPortProxies with that lifecycle-managed context from Start; then make
Stop() cancel it and clear the running state so only one watcher exists per
runtime instance.
---
Nitpick comments:
In `@backend/internal/api/handlers.go`:
- Around line 264-292: The validateNoProxyEntry function in handlers.go is
redundantly calling net.SplitHostPort twice on the same host value. Refactor
that block so the host and port are extracted once, then reuse the parsed host
for the net.ParseIP and isValidDomain checks; keep the existing validation
behavior and error messages unchanged.
In `@backend/internal/api/networks.go`:
- Around line 16-24: The same runtime-error fallback pattern is duplicated in
this file, so extract it into a small helper on the handler type, such as
s.writeRuntimeOrInternalError(w, err), and use it in the ListNetworks flow and
the other repeated runtime calls. Keep the helper’s behavior identical to the
current writeRuntimeError followed by writeError(..., err.Error()) fallback so
the existing methods remain unchanged while removing duplication.
In `@backend/internal/buildhistory/inspect.go`:
- Around line 103-109: The stripLastComponent helper is manually reimplementing
directory extraction; replace the string-based slash handling with the stdlib
path.Dir function in stripLastComponent. Add/import path (not path/filepath) so
Unix-style container path semantics are preserved, and keep the function
behavior aligned with the current callers in inspect.go.
In `@backend/internal/runtime/mock.go`:
- Around line 662-675: `ListNetworks` and `RemoveNetwork` are incorrectly
sharing container-specific error fields, so network test cases can interfere
with container scenarios. Update the `Mock` runtime to use dedicated network
error fields (for example `NetworksErr` and `NetworkErr`) in the `ListNetworks`
and `RemoveNetwork` methods, and wire those new fields through the mock struct
and any related setup so `ContainersErr`/`ContainerErr` remain container-only.
In `@backend/internal/runtime/network.go`:
- Around line 249-265: networkInspectMetadataBatch currently swallows every
inspectNetworkMetadata failure and returns nil, which hides per-network
enrichment problems. Update networkInspectMetadataBatch to track failed network
names and surface the specific inspectNetworkMetadata error for each one, either
by returning a partial/aggregated error or logging the failure with the network
name. Then adjust listNetworks to handle the returned error path meaningfully
instead of relying on the dead err check, using the existing
inspectNetworkMetadata and networkInspectMetadataBatch symbols to keep failures
identifiable.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 81700c26-6b34-4dd1-a65e-6198af111993
📒 Files selected for processing (29)
.cursor/rules/calf.mdcCHANGELOG.mdCLAUDE.mdROADMAP.mdbackend/cmd/calf/main.gobackend/internal/api/handlers.gobackend/internal/api/networks.gobackend/internal/api/server.gobackend/internal/buildhistory/inspect.gobackend/internal/config/config.gobackend/internal/config/config.yamlbackend/internal/runtime/errors.gobackend/internal/runtime/lima.gobackend/internal/runtime/lima.yamlbackend/internal/runtime/localhost_proxy.gobackend/internal/runtime/mock.gobackend/internal/runtime/native.gobackend/internal/runtime/network.gobackend/internal/runtime/proxy.gobackend/internal/runtime/runtime.gobackend/test/api/api_test.gobackend/test/runtime/network_test.gobackend/version/version.goui/lib/api/client.dartui/lib/app_shell.dartui/lib/screens/networks_screen.dartui/lib/screens/resources_screen.dartui/pubspec.yamlui/test/widget_test.dart
Comment on lines
+81
to
+101
| func resolveContextFromLabels(labels map[string]string) string { | ||
| if workingDir, ok := labels["com.docker.compose.project.working_dir"]; ok && workingDir != "" { | ||
| if _, err := os.Stat(workingDir); err == nil { | ||
| return workingDir | ||
| } | ||
| } | ||
|
|
||
| if configFiles, ok := labels["com.docker.compose.project.config_files"]; ok { | ||
| for _, part := range strings.Split(configFiles, ",") { | ||
| part = strings.TrimSpace(part) | ||
| if part == "" { | ||
| continue | ||
| } | ||
| if _, err := os.Stat(part); err == nil { | ||
| return stripLastComponent(part) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return "" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how Lima mounts the host filesystem into the VM (path parity assumption)
rg -n -A5 -B5 'mounts|writable|hostpath' backend/internal/runtime/lima.yamlRepository: enegalan/calf
Length of output: 364
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== inspect.go ==\n'
ast-grep outline backend/internal/buildhistory/inspect.go --view expanded || true
wc -l backend/internal/buildhistory/inspect.go
printf '\n== relevant slice ==\n'
sed -n '1,220p' backend/internal/buildhistory/inspect.go
printf '\n== runtime files ==\n'
fd -a '.*' backend/internal/runtime | sed -n '1,200p'
printf '\n== search for compose labels / build history ==\n'
rg -n 'com\.docker\.compose\.project\.(working_dir|config_files)|resolveContextFromLabels|buildhistory|working_dir|config_files' backend -SRepository: enegalan/calf
Length of output: 7094
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== runtime/lima.go ==\n'
wc -l backend/internal/runtime/lima.go
sed -n '1,240p' backend/internal/runtime/lima.go
printf '\n== runtime/lima.yaml ==\n'
sed -n '1,220p' backend/internal/runtime/lima.yaml
printf '\n== compose migration / label sources ==\n'
sed -n '1,220p' backend/internal/migration/compose_migration.go
printf '\n== buildhistory inspect tests ==\n'
wc -l backend/internal/buildhistory/inspect_test.go
sed -n '1,240p' backend/internal/buildhistory/inspect_test.goRepository: enegalan/calf
Length of output: 15142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== native runtime ==\n'
wc -l backend/internal/runtime/native.go
sed -n '1,260p' backend/internal/runtime/native.go
printf '\n== docker socket / path mapping helpers ==\n'
rg -n 'VMPath|HostPath|dockerSocket|mountPoint|/mnt/calf|/mnt/lima-cidata|mounted|forward' backend/internal/runtime backend/internal/migration -SRepository: enegalan/calf
Length of output: 10463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== runDocker / socket plumbing ==\n'
rg -n 'func runDocker|defaultDockerSocket|DockerSocket\(\)|runDocker\(' backend/internal -S
printf '\n== buildhistory files ==\n'
sed -n '1,220p' backend/internal/buildhistory/history.go
sed -n '1,220p' backend/internal/buildhistory/logs.goRepository: enegalan/calf
Length of output: 8491
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== build path handling ==\n'
rg -n 'RunBuild|buildx|compose|working_dir|config_files|Context' backend/internal/runtime backend/internal/api backend/internal/migration -S
printf '\n== defaultDockerSocket / Lima socket ==\n'
sed -n '620,680p' backend/internal/runtime/lima.go
printf '\n== VMPath helper ==\n'
sed -n '110,150p' backend/internal/migration/docker_desktop.goRepository: enegalan/calf
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== defaultDockerSocket + Lima path helpers ==\n'
sed -n '620,675p' backend/internal/runtime/lima.go
sed -n '150,210p' backend/internal/runtime/build_enrich.go
sed -n '1,220p' backend/internal/api/builds.go
printf '\n== VMPath helper and compose mount root ==\n'
sed -n '110,145p' backend/internal/migration/docker_desktop.go
sed -n '540,575p' backend/internal/migration/docker_desktop.goRepository: enegalan/calf
Length of output: 8825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== runBuild implementations ==\n'
sed -n '70,130p' backend/internal/runtime/nerdctl.go
sed -n '330,390p' backend/internal/runtime/mock.go
sed -n '330,380p' backend/internal/runtime/native.go
sed -n '330,380p' backend/internal/runtime/lima.go
printf '\n== build result enrichment / path resolution ==\n'
sed -n '1,120p' backend/internal/runtime/build_enrich.go
sed -n '120,210p' backend/internal/runtime/build_enrich.go
printf '\n== runtime build struct ==\n'
sed -n '1,120p' backend/internal/runtime/build.goRepository: enegalan/calf
Length of output: 13630
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== build job and source-path resolution ==\n'
sed -n '220,420p' backend/internal/api/builds.go
printf '\n== build source / staging helpers ==\n'
rg -n 'stage|staging|mountsRoot|VMPath|RunBuild\\(|Context:' backend/internal/api backend/internal/migration backend/internal/runtime -SRepository: enegalan/calf
Length of output: 5730
Translate Lima build-history paths before os.Stat. Native works as-is, but Lima records compose labels as VM paths (for example /mnt/calf/...), while this check runs on the host. That makes resolveContextFromLabels drop valid build contexts for Lima-backed builds. Map the label back to the host mount root before checking.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/buildhistory/inspect.go` around lines 81 - 101,
resolveContextFromLabels is checking compose label paths directly with os.Stat,
which fails for Lima VM paths. Update the path handling in
resolveContextFromLabels to translate com.docker.compose.project.working_dir and
com.docker.compose.project.config_files from the VM path to the host mount root
before stat’ing them, then return the translated working directory or
stripLastComponent from the translated config file path.
Comment on lines
+15
to
+62
| func applyProxyInVM(ctx context.Context, run commandRunner, proxy ProxyConfig) error { | ||
| httpProxy := shellQuote(proxy.HTTPProxy) | ||
| httpsProxy := shellQuote(proxy.HTTPSProxy) | ||
| noProxy := shellQuote(proxy.NoProxy) | ||
|
|
||
| script := fmt.Sprintf(`set -eux -o pipefail | ||
| sudo mkdir -p /etc/systemd/system/containerd.service.d | ||
| sudo tee /etc/systemd/system/containerd.service.d/calf-proxy.conf >/dev/null <<'EOF' | ||
| [Service] | ||
| Environment="HTTP_PROXY=%s" | ||
| Environment="HTTPS_PROXY=%s" | ||
| Environment="NO_PROXY=%s" | ||
| Environment="http_proxy=%s" | ||
| Environment="https_proxy=%s" | ||
| Environment="no_proxy=%s" | ||
| EOF | ||
| sudo tee /etc/profile.d/calf-proxy.sh >/dev/null <<'EOF' | ||
| export HTTP_PROXY=%s | ||
| export HTTPS_PROXY=%s | ||
| export NO_PROXY=%s | ||
| export http_proxy=%s | ||
| export https_proxy=%s | ||
| export no_proxy=%s | ||
| EOF | ||
| sudo systemctl daemon-reload | ||
| if systemctl is-active --quiet containerd; then | ||
| sudo systemctl restart containerd | ||
| fi | ||
| if systemctl is-active --quiet docker; then | ||
| sudo systemctl restart docker | ||
| fi | ||
| `, | ||
| httpProxy, httpsProxy, noProxy, httpProxy, httpsProxy, noProxy, | ||
| httpProxy, httpsProxy, noProxy, httpProxy, httpsProxy, noProxy, | ||
| ) | ||
|
|
||
| _, err := run(ctx, "bash", "-lc", script) | ||
| return err | ||
| } | ||
|
|
||
| func shellQuote(value string) string { | ||
| value = strings.TrimSpace(value) | ||
| if value == "" { | ||
| return "" | ||
| } | ||
|
|
||
| return strings.NewReplacer(`'`, `'"'"'`).Replace(value) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files | rg '^(backend/internal/runtime/proxy\.go|backend/internal/runtime/.*\.go|backend/internal/.*\.go)$'
printf '\n== Outline proxy.go ==\n'
ast-grep outline backend/internal/runtime/proxy.go --view expanded || true
printf '\n== Relevant call sites ==\n'
rg -n "applyProxyInVM|ApplyProxy|ProxyConfig|HTTPProxy|HTTPSProxy|NoProxy|shellQuote" backend/internal/runtime backend/internal -g '*.go'
printf '\n== Candidate validation/config definitions ==\n'
rg -n "type ProxyConfig|struct .*Proxy|Validate\\(|validation|proxy" backend/internal -g '*.go'Repository: enegalan/calf
Length of output: 15776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Read proxy.go with line numbers ==\n'
sed -n '1,220p' backend/internal/runtime/proxy.go | cat -n
printf '\n== Read native/lima runtime callers ==\n'
for f in backend/internal/runtime/native.go backend/internal/runtime/lima.go backend/internal/runtime/mock.go; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,240p' "$f" | cat -n
fi
doneRepository: enegalan/calf
Length of output: 23679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search for proxy validation / sanitization in API and models ==\n'
rg -n "HTTPProxy|HTTPSProxy|NoProxy|ProxyConfig|proxy.*validate|validate.*proxy|TrimSpace\\(|NewReplacer|strings\\.ReplaceAll\\(.*\\n" backend -g '*.go'
printf '\n== Search for write paths to proxy settings ==\n'
rg -n "ApplyProxy\\(|applyProxyInVM\\(|proxy settings|proxy config" backend -g '*.go'Repository: enegalan/calf
Length of output: 411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Proxy validation in API handlers ==\n'
sed -n '100,310p' backend/internal/api/handlers.go | cat -n
printf '\n== Proxy config schema ==\n'
sed -n '1,120p' backend/internal/config/config.go | cat -nRepository: enegalan/calf
Length of output: 9944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Where validateProxyUpdate is called ==\n'
rg -n "validateProxyUpdate\\(" backend/internal/api/handlers.go backend/internal/api -g '*.go'
printf '\n== Rest of handlers.go around validator ==\n'
sed -n '1,240p' backend/internal/api/handlers.go | cat -nRepository: enegalan/calf
Length of output: 8679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat >/tmp/proxy_url_probe.go <<'EOF'
package main
import (
"fmt"
"net/url"
)
func main() {
tests := []string{
"http://proxy.example.com",
"http://proxy.example.com/?q=$(id)",
"http://proxy.example.com/?q=foo;bar",
"http://proxy.example.com/?q=foo bar",
"http://proxy.example.com/?q=foo\nbar",
}
for _, raw := range tests {
u, err := url.Parse(raw)
if err != nil {
fmt.Printf("PARSE_ERR\t%q\t%v\n", raw, err)
continue
}
fmt.Printf("PARSE_OK\t%q\tscheme=%q host=%q path=%q rawquery=%q\n", raw, u.Scheme, u.Host, u.Path, u.RawQuery)
}
}
EOF
go run /tmp/proxy_url_probe.goRepository: enegalan/calf
Length of output: 727
Escape proxy values before interpolating them into the shell script. validateProxyURL() still accepts proxy URLs with shell metacharacters in the query/fragment (for example http://proxy.example.com/?q=$(id) or spaces), and shellQuote() doesn’t make these substitutions shell-safe. On the Native path this runs through bash -lc on the host, so a crafted proxy value can execute commands and persist into /etc/profile.d/calf-proxy.sh and the containerd drop-in.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/runtime/proxy.go` around lines 15 - 62, The proxy values
interpolated by applyProxyInVM are not fully shell-safe, so malicious characters
can still be executed through the bash -lc script. Fix shellQuote to return a
properly shell-escaped value that handles spaces and shell metacharacters, and
ensure applyProxyInVM uses that escaped output consistently for every
HTTPProxy/HTTPSProxy/NoProxy insertion in both the containerd drop-in and
/etc/profile.d script.
…rk operations - Updated network handling methods to use context.Background() for better control over timeouts. - Introduced a new helper function for consistent error handling in network-related API responses. - Enhanced proxy application logic to log warnings for non-fatal errors during startup. - Refactored network inspection and removal methods to improve error reporting and handling. - Updated tests to reflect changes in error handling and context management.
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
1 participant
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