Share the CEL environment, let callers extend it, and add it to SSH - #1116
Closed
joshdrake wants to merge 1 commit into
Closed
Share the CEL environment, let callers extend it, and add it to SSH#1116joshdrake wants to merge 1 commit into
joshdrake wants to merge 1 commit into
Conversation
The cel function built a fresh environment inside WithTemplate, so every
certificate paid to register the native types and initialise seven
extension libraries: 141.8us and 286KB, including for the templates that
contain no cel call at all, which today is all of them. The environment
does not depend on the certificate, only the activation does, so it is
now built once and shared. Compiled programs are cached with it, since a
template's expressions do not change between signatures.
The environment could also only ever be as good as this library's
knowledge of the data. Subject and SANs have types here; a webhook
response does not, so it is dyn and the checker cannot see through it --
which is unfortunate, because that is where a CA keeps everything
interesting. celutil.Register lets a caller declare typed variables and
supply their values from the template data, so an expression can read
device.serial as a string instead of Webhooks.Agent.Device.Serial as a
dyn, and a misspelled field is a compile error rather than a certificate
that renders wrongly. One registration covers X.509 and SSH alike.
The function now returns the result as a value rather than formatting it
to a string, so a template can pipe it: {{ cel "..." | toJson }} is
correct for a string, a list or a number. Formatting first made toJson
produce a quoted string for every type, which is wrong everywhere a
template needs a list. The one visible consequence is that a list
rendered bare prints as [a b c] rather than [a, b, c].
Also: SSH templates gain the function, which they did not have;
CELEnvOptions is exported so a caller validating expressions ahead of
time can build the environment the renderer will actually use rather
than approximating it; the cost limit is settable, and changing it
discards programs compiled under the old one; and the top-level CR
variable is dropped, having been declared but never populated -- a
template referencing it compiled and then failed at signing time.
Change-Type: refactor
Release-Note: yes
Audience: developer
Impact: low
Breaking: false
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016N6LAjGgci2KLKYDbdWVcw
|
|
Contributor
Author
|
Superseded. Rather than a public CEL environment in this library, crypto gets a single seam for contributing a template function and the CEL machinery moves to the caller. Replacement PR to follow. |
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
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.
Builds on #1115. Base is
mariano/cel, so this diff is just the changes on top.The environment is built once, not per certificate
newCelFuncran insideWithTemplate, so every certificate paid to register the native types and initialise seven extension libraries — 141.8µs and 286KB — including for templates containing nocelcall at all, which today is all of them. The environment doesn't depend on the certificate; only the activation does. It's now built at most once and shared, with compiled programs cached alongside it since a template's expressions don't change between signatures.The no-cel-call case is the pre-#1115 baseline again.
Callers can declare typed variables
The environment can only ever be as good as this library's knowledge of the data.
SubjectandSANshave types here; a webhook response doesn't, so it'sdynand the checker can't see through it — which is unfortunate, because that's where a CA keeps everything interesting.celutil.Registerlets a caller declare typed variables and supply their values from the template data:An expression then reads
device.serialas astringinstead ofWebhooks.Agent.Device.Serialas adyn, and a misspelled field is a compile error rather than a certificate that renders wrongly. One registration covers X.509 and SSH alike. Registering invalidates cached environments rather than being silently ignored, so there's no ordering trap.The function returns a value
fmt.Sprint(out)made the result unusable in a JSON position: a string rendered bare (example, not"example") and a list rendered as[a, b, c]. Piping totoJsondidn't fix it either — correct for a string, and it quietly turns a list into a JSON string. Returning the value means{{ cel "..." | toJson }}is correct for a string, a list or a number.The one visible consequence: a list rendered bare now prints
[a b c]rather than[a, b, c]. Thesanstest expectation is updated for that.Also
sshutilhas its owngetFuncMapthat Add a cel function to X.509 templates #1115 didn't touch.CELEnvOptionsis exported on both packages, so a caller validating expressions ahead of time builds the environment the renderer will actually use rather than approximating it.CRvariable is dropped — it was declared but never populated (the data puts it atInsecure.CR), so an expression referencing it compiled and then failed at signing time.Note on
json.encodeand the cost limitWorth knowing independently of this PR: cel-go declares
json.encodewith an unbounded cost estimate, and when its argument is a literal the cost tracker charges that estimate rather than the real work. Sojson.encode("wifi")is metered at 2^64-1 and tripsCostLimit(1000):Any template doing
{{cel "json.encode({'a': 'b'})"}}fails to sign today. Returning a value sidesteps it for the common case (use| toJsoninstead), but it's a real trap for anyone reaching forjson.encodein a template.Full
go test ./...passes.Related
expressionfield🤖 Generated with Claude Code
https://claude.ai/code/session_016N6LAjGgci2KLKYDbdWVcw