Supply WENDY_JETPACK_MAJOR as docker build arg - #1144
Conversation
There was a problem hiding this comment.
Note
Integration test review — automated suggestions from Claude. Apply, adapt, or dismiss as needed.
NO_CHANGES_NEEDED
The PR introduces WENDY_JETPACK_MAJOR as a new docker build arg derived from WENDY_JETPACK_VERSION. This is a build-time argument injection feature specific to Jetson hardware — it is already covered by a new Go unit test (TestApplyDeviceBuildArgHints_DerivesJetpackMajor) in device_build_args_test.go that exercises both the happy path ("7.2" → "7") and the L4T fallback omission case. An integration test for this feature would require a physical Jetson device with a specific JetPack version, and the feature itself has no observable runtime behavior (it's a build arg consumed at image-build time, not at container runtime). There is no new entitlement, deployment mode, or CLI command introduced. No integration test additions are warranted.
AI Security Review |
| Severity | Standards | File | Line(s) | Title |
|---|---|---|---|---|
| LOW | [SOC2-CC7] [ISO27001-A.12] | go/internal/cli/commands/docker.go |
~309–315 | jetpackMajor silently drops malformed version strings without logging |
| LOW | [ISO27001-A.8] [NIST-SI] | go/internal/cli/commands/docker.go |
~307 | Device-reported version string flows into build args without an explicit length cap |
| INFORMATIONAL | [SOC2-CC8] | go/internal/cli/commands/device_build_args_test.go |
~62–73 | Test coverage gap: no test for boundary/edge inputs to jetpackMajor directly |
| INFORMATIONAL | [ISO27001-A.8] | go/internal/cli/commands/docker.go |
~309 | strings.Cut on an empty string yields ("", "", false) — benign but implicit |
3. Detailed Findings
Finding 1 — jetpackMajor silently drops malformed version strings without logging
Severity: LOW
Standards: [SOC2-CC7] [ISO27001-A.12]
Description:
When jetpackMajor receives a version string that does not parse as a clean <major>.<rest> numeric form (e.g., the documented "L4T 39.2.0" fallback, or any future unexpected format), it silently returns "". The caller setHint in turn silently skips the key when the value is empty. The existing setHint path does emit a warning when validateBuildArgPair rejects a value, but jetpackMajor short-circuits before that validation path is ever reached for bad inputs. This means operator visibility of unusual agent-reported version strings is reduced — a device reporting a novel or corrupted version string will simply omit the build arg with no diagnostic output.
Relevant snippet:
func jetpackMajor(version string) string {
major, _, _ := strings.Cut(version, ".")
if _, err := strconv.Atoi(major); err != nil {
return "" // empty, or an unmapped "L4T 39.2.0" fallback — not a clean major
}
return major
}Remediation:
Add a log.Debugf or equivalent trace-level log statement in the err != nil branch when version is non-empty (i.e., the agent did report something, but it wasn't parseable). This preserves the silent-omit behaviour for build correctness while giving operators visibility. Example:
func jetpackMajor(version string) string {
if version == "" {
return ""
}
major, _, _ := strings.Cut(version, ".")
if _, err := strconv.Atoi(major); err != nil {
log.Debugf("jetpackMajor: skipping non-numeric JetPack version %q", version)
return ""
}
return major
}Controls violated: SOC2 CC7.2 (anomaly detection / operational visibility), ISO 27001 A.12.4 (event logging).
Finding 2 — Device-reported version string flows into build args without an explicit length cap
Severity: LOW
Standards: [ISO27001-A.8] [NIST-SI]
Description:
versionResp.GetJetpackVersion() is a device-reported string. It is passed first to jetpackMajor, which calls strings.Cut and strconv.Atoi on a potentially unbounded input, and the result is subsequently passed to setHint → validateBuildArgPair for regex validation. While strconv.Atoi and strings.Cut are not vulnerable to catastrophic behaviour on large inputs in Go, the existing validBuildArgValueRe regex (not shown in this diff but presumably anchored) is the primary guard. There is no explicit upper-bound check on the raw version string length before processing begins.
For a defence-in-depth posture — particularly given the comment that these values are "device-reported and feed straight into a builder CLI" — an explicit length guard would eliminate any theoretical DoS or regex-amplification risk if the regex is ever modified.
Relevant snippet:
setHint("WENDY_JETPACK_MAJOR", jetpackMajor(versionResp.GetJetpackVersion()))Remediation:
Add a length cap early in jetpackMajor (or centrally in setHint) before any string processing:
func jetpackMajor(version string) string {
if len(version) > 64 { // generous cap; real versions are < 10 chars
return ""
}
// ... existing logic
}Alternatively, enforce a maximum length inside setHint or validateBuildArgPair that applies uniformly to all hints.
Controls violated: ISO 27001 A.8.28 (secure coding — input validation), NIST SP 800-53 SI-10 (information input validation).
Finding 3 — Test coverage gap: no direct unit tests for jetpackMajor edge cases
Severity: INFORMATIONAL
Standards: [SOC2-CC8]
Description:
The new test TestApplyDeviceBuildArgHints_DerivesJetpackMajor exercises two cases via the higher-level applyDeviceBuildArgHints wrapper: a clean "7.2" input and the "L4T 39.2.0" fallback. However, jetpackMajor itself is not tested directly for edge cases such as: empty string "", a bare integer with no dot "7", a multi-segment version "7.2.1", a version with leading zeroes "07.2", or an excessively long/malformed string. Lack of direct tests for the helper makes future refactors riskier.
Relevant snippet:
func TestApplyDeviceBuildArgHints_DerivesJetpackMajor(t *testing.T) {
clean := map[string]string{}
applyDeviceBuildArgHints(clean, &agentpb.GetAgentVersionResponse{JetpackVersion: strptr("7.2")})
// ...
fallback := map[string]string{}
applyDeviceBuildArgHints(fallback, &agentpb.GetAgentVersionResponse{JetpackVersion: strptr("L4T 39.2.0")})
// ...
}Remediation:
Export or add a package-level test for jetpackMajor directly (or via a table-driven test within the _test.go file using //go:build unexported access), covering: empty string, no-dot input, multi-segment input, leading-zero input, and an oversized string if a cap is added.
Controls violated: SOC2 CC8.1 (change management — adequate testing prior to deployment).
Finding 4 — strings.Cut on empty string is implicitly safe but undocumented
Severity: INFORMATIONAL
Standards: [ISO27001-A.8]
Description:
When versionResp.GetJetpackVersion() returns "" (agent did not report a JetPack version), jetpackMajor("") calls strings.Cut("", "."), which returns ("", "", false). Then strconv.Atoi("") returns an error, so "" is returned — which setHint correctly skips. This is the desired behaviour, but it is reached via the error-path of Atoi rather than an explicit empty-string guard. A future reader might find this non-obvious.
Relevant snippet:
func jetpackMajor(version string) string {
major, _, _ := strings.Cut(version, ".")
if _, err := strconv.Atoi(major); err != nil {
return ""
}
return major
}Remediation:
Add an explicit early-return for the empty-string case (as shown in Finding 1's remediation) for clarity and to make the intent self-documenting. No functional change required.
Controls violated: ISO 27001 A.8.28 (secure coding practices — code clarity/maintainability).
4. Compliance Summary
| Framework | Checked | Violations / Risks Found |
|---|---|---|
| SOC 2 (TSC) | ✅ | LOW: CC7 (reduced operational visibility on malformed inputs); INFORMATIONAL: CC8 (test coverage) |
| ISO/IEC 27001:2022 | ✅ | LOW: A.8 (input validation, length cap); INFORMATIONAL: A.8 (code clarity) |
| PCI DSS v4.0 | N/A | Diff does not touch payment or card data flows — not assessed |
| GDPR / Privacy | ✅ | No personal data involved — no violations found |
| HIPAA | N/A | Diff does not touch health or medical data — not assessed |
| NIST SP 800-53 / CSF 2.0 | ✅ | LOW: SI-10 (input validation on device-reported string) |
Overall assessment: This is a low-risk, narrowly scoped change. No critical or high severity issues were identified. The two LOW findings (missing debug log on parse failure, missing length cap on device-reported input) are defence-in-depth recommendations rather than exploitable vulnerabilities in typical deployment contexts. Addressing Finding 1 (adding a debug-level log) is the most actionable improvement before merge.
There was a problem hiding this comment.
Pull request overview
Adds a coarse JetPack selector build arg (WENDY_JETPACK_MAJOR) to make it easier for Dockerfiles/Containerfiles to choose base images by JetPack generation (e.g., 6 vs 7) while preserving the existing fine-grained version args.
Changes:
- Inject
WENDY_JETPACK_MAJORas a validated build-arg hint derived fromWENDY_JETPACK_VERSION. - Add unit coverage ensuring the major is derived for clean versions and omitted for unmapped
L4T ...fallbacks. - Document the new build arg for both
wendy runandwendy build.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| go/internal/cli/commands/run.go | Updates Jetson platform-selection guidance to mention WENDY_JETPACK_MAJOR. |
| go/internal/cli/commands/docker.go | Derives and injects WENDY_JETPACK_MAJOR during build-arg hint application. |
| go/internal/cli/commands/device_build_args_test.go | Adds tests for WENDY_JETPACK_MAJOR derivation/omission behavior. |
| go/internal/cli/assets/docs/clients/wendy-cli/commands/run.md | Documents WENDY_JETPACK_MAJOR in wendy run build-arg table. |
| go/internal/cli/assets/docs/clients/wendy-cli/commands/build.md | Documents WENDY_JETPACK_MAJOR in wendy build build-arg table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // applyDeviceBuildArgHints injects the optional device/GPU build-arg hints the | ||
| // agent reports (WENDY_DEVICE_TYPE, WENDY_HAS_GPU, WENDY_GPU_VENDOR, | ||
| // WENDY_JETPACK_VERSION, WENDY_CUDA_VERSION) into buildArgs. Each hint is only | ||
| // set when the agent reports it, so Dockerfiles keep their own ARG defaults on | ||
| // older agents. These values are device-reported and feed straight into a | ||
| // builder CLI, so any hint that fails build-arg validation is skipped with a | ||
| // warning rather than failing the whole deploy — e.g. a Jetson running an L4T | ||
| // release the agent's JetPack table doesn't map reports a fallback like | ||
| // "L4T 38.2.0", whose space is rejected by validBuildArgValueRe. | ||
| // WENDY_JETPACK_VERSION, WENDY_JETPACK_MAJOR, WENDY_CUDA_VERSION) into | ||
| // buildArgs. Each hint is only set when the agent reports it, so Dockerfiles | ||
| // keep their own ARG defaults on older agents. These values are device-reported |
Docs previewPreview this PR's docs at: https://docs.wendy.dev/branch-thombles-jetpack-major-da086b6fc508d573158446e1c0ed23e0a6309a83/ This comment is updated automatically when the docs preview is redeployed. |
For some apps it's useful to select a base image based on whether we're dealing with Jetpack 6 vs Jetpack 7. While we already have
WENDY_JETPACK_VERSION, this is a value like7.2which is not easily used for conditional selection within aDockerfile. If we provideWENDY_JETPACK_MAJOR(7) then this is straightforward.Example usage in paired PR: wendylabsinc/samples#14