examples: sweep /v1/svid + /v1/attest/k8s payload build to Python argv#58
Conversation
Follow-up to the gemini comment on PR #57, applied to the older sibling demos for consistency. examples/ca-vault-pki/run-demo.sh and examples/k8s-attest/run-demo.sh both used the same pattern: interpolate a shell variable (a path or a token) into a Python source-string, then stitch the resulting JSON literal directly into a curl body. Both moves are fragile - a quote, single quote, or backslash in the value can break the Python parse or break out of the curl body. Move both to the safer shape: pass values to Python via argv and let json.dumps build the whole object. examples/k8s-attest gets a small build_attest_payload helper because it makes two attest calls (allow + deny) with different tokens and the same CSR. Verified locally: - examples/ca-vault-pki: `make demo` → `[demo] success` - examples/k8s-attest: `make demo` → `[demo] success` (cold) and `KEEP_CLUSTER=1 make demo` → `[demo] success` (warm rerun)
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the security and reliability of JSON payload construction in demo shell scripts by utilizing Python's json.dumps and sys.argv to mitigate shell injection risks. The reviewer recommended assigning the payload to a variable before executing curl to ensure that any failures in the subshell are properly handled by set -e. Furthermore, the feedback suggests using the -S flag with curl to ensure that error messages are not suppressed during execution.
Two related gemini mediums on the run-demo I introduced in this PR.
Both apply:
- Under `set -e`, a command substitution inside an argument (`curl
-d "$(build_attest_payload ...)"`) does not abort the script when
the substitution fails. If the Python helper errors (missing CSR,
bad token), curl quietly POSTs an empty body and the downstream
assertion ends up blaming the server instead of the real cause.
Assign the payload to a variable first so a failure surfaces here.
- The wrong-audience branch used `curl -s -o ... -w '%{http_code}'`
to read just the status. `-s` also swallows curl's own error
messages (network drop, DNS failure, broken TLS), which would
leave the script stuck with no diagnostic. `-sS` keeps the body
silent but lets curl's own stderr through.
Verified locally: `make demo` cold run → `[demo] success`.
1 participant
Summary
Follow-up to gemini's comment on PR #57. The fix landed there for
examples/ca-step-ca/; this PR applies the same safer pattern to the two older sibling demos that had the same shape:examples/ca-vault-pki/run-demo.shexamples/k8s-attest/run-demo.shBoth used the same fragile combo: interpolate a shell variable (path or token) into a Python source-string, then stitch the resulting JSON literal directly into a curl body. A quote, single quote, or backslash in the value can break the Python parse or break out of the curl body. None of those values are user-controlled today, so the original behaviour was correct in practice — but the new shape is the one to copy when these scripts grow or get reused.
Move both to the same pattern as PR #57: pass values to Python via argv and let
json.dumpsbuild the whole object.examples/k8s-attestgets a smallbuild_attest_payloadhelper because it makes two attest calls (allow + deny) with different tokens and the same CSR.Scope layer
Plugin / example. No production code changes. Tightens a pattern across two scripts.
Test plan
examples/ca-vault-pki:make demo→[demo] successexamples/k8s-attest: coldmake demo→[demo] successexamples/k8s-attest:KEEP_CLUSTER=1 make demo(warm rerun) →[demo] successkind-k8s-attestjob re-runs the demos on every push.