Skip to content

docs: note 1MB JSON payload limit and upgrade x/crypto - #1148

Merged
sami-alajrami merged 5 commits into
mainfrom
docs/attestation-data-1mb-note
Sep 3, 2026
Merged

docs: note 1MB JSON payload limit and upgrade x/crypto#1148
sami-alajrami merged 5 commits into
mainfrom
docs/attestation-data-1mb-note

Conversation

@sami-alajrami

@sami-alajrami sami-alajrami commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Documents the 1MB maximum JSON payload size in the --attestation-data (custom attestation) and --user-data flag descriptions
  • Updates the generated docs golden file to match
  • Upgrades golang.org/x/crypto to v0.56.0, fixing CVE-2026-56855 (SNYK-GOLANG-GOLANGORGXCRYPTOSSH-19504090) flagged by the Snyk CI scan — a resource-exhaustion vulnerability in x/crypto/ssh connection multiplexing

Related to kosli-dev/server#5388

Test plan

  • go build ./...
  • go test ./cmd/kosli/... -run TestDocsCommandTestSuite -v

@github-actions github-actions Bot added go Pull requests that update go code docs labels Sep 3, 2026
@claude

claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sami-alajrami's task in 2m 17s —— View job


Review: docs 1MB note + x/crypto bump

Both earlier nits are resolved on this push — trailUserDataFlag now carries the note, and all three strings use uppercase JSON consistently. No blocking issues.

Phase 1 — static analysis

Check Result
go build ./... ✅ pass
go vet ./... ✅ pass
go mod tidygit diff go.mod go.sum ✅ clean, no drift

I couldn't run go test ./cmd/kosli/... in this environment (command not in the allowed tool list), so I verified golden-file coverage statically instead — see below.

Phase 2 — findings

Golden-file coverage is complete ✅ — I grepped the whole repo for the three changed strings:

cmd/kosli/root.go
cmd/kosli/testdata/output/docs/mintlify/snyk.md

cmd/kosli/testdata/output/docs/mintlify/ contains only artifact.md and snyk.md, and artifact.md doesn't reference any of them. So snyk.md was the only golden file needing regeneration — nothing else is stale. empty-flag-audit-coverage.json keys off flag names, not usage strings, so it's unaffected.

No client-side enforcement of the documented limit (cmd/kosli/cli_utils.go:439-458) — nit, non-blocking. LoadJsonData validates that the content is JSON but never checks size, so a 2MB file is read, parsed, and sent, and the user learns about the cap from a server-side rejection rather than from the CLI. Documenting first is the right thin slice; if you want the friendlier error, a follow-up slice could add the check. Two caveats worth weighing before doing so: the cap lives server-side (kosli-dev/server#5388), so a hardcoded client constant can silently drift if the server value ever changes, and the CLI limit would need to account for the fact that the request body is larger than the file alone.

"1MB" is unit-ambiguous — nit. 1,000,000 vs 1,048,576 bytes. Only matters for users sitting right on the boundary, and it matches however the server-side docs phrase it, so I'd leave it unless the server issue is more precise.

Dependency bump bundled with a docs change — worth a note only because go.mod/go.sum in a docs PR is easy to skim past. The PR body explains it (Snyk CI gate on CVE-2026-56855), the title and go label both flag it, and it's a clean patch bump with no tidy drift, so this is fine as-is. One thing I couldn't confirm: x/crypto is an indirect dependency here, so whether x/crypto/ssh is actually reachable from this binary — and thus whether the CVE was ever exploitable versus just tripping the scanner — is unverified (go mod why wasn't available to me). Doesn't change the outcome; unblocking CI is reason enough.

What's done well

Nice catch extending the note to trailUserDataFlag rather than only fixing the flagged line — beginTrail.go:96 goes through the same LoadJsonDatauser_data path, so the two flags would have documented the same server behaviour inconsistently. Regenerating the golden file in the same commit instead of leaving it for CI to catch is the right habit, and the casing cleanup on attestationCustomDataFileFlag makes all three strings read the same way.

LGTM.
· Branch: docs/attestation-data-1mb-note

Comment thread cmd/kosli/root.go Outdated
Comment thread cmd/kosli/root.go Outdated
Fixes CVE-2026-56855 (SNYK-GOLANG-GOLANGORGXCRYPTOSSH-19504090), a
resource-exhaustion vulnerability in x/crypto/ssh connection
multiplexing that could let a malicious peer deadlock a connection.
@sami-alajrami sami-alajrami changed the title docs: note 1MB max JSON payload size for --attestation-data and --user-data docs+deps: attestation-data note and x/crypto upgrade Sep 3, 2026
sami-alajrami and others added 2 commits September 3, 2026 10:56
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@sami-alajrami sami-alajrami changed the title docs+deps: attestation-data note and x/crypto upgrade docs: note 1MB JSON payload limit and upgrade x/crypto Sep 3, 2026
Comment thread cmd/kosli/root.go Outdated
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@sami-alajrami
sami-alajrami merged commit 5054581 into main Sep 3, 2026
20 checks passed
@sami-alajrami
sami-alajrami deleted the docs/attestation-data-1mb-note branch September 3, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs go Pull requests that update go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants