Builtin functions should not have access to StepsContext
Summary
Closes #437 (closed)
Builtin functions previously received *runner.StepsContext directly, giving them access to previously executed step results via StepResults(), RecordResult(), View().StepResults, and other internal state-management methods. This violates isolation — builtins should only see their own pre-expanded inputs, environment, and file paths.
This MR introduces a BuiltinContext interface with a standalone builtinContext struct that snapshots only the data builtins need. The struct holds copied values (not a pointer to StepsContext), so builtins cannot reach step results even via reflection.
Changes
- New
BuiltinContextinterface (pkg/runner/builtin_context.go) — exposes 11 methods: input access, environment, working directories, I/O pipes, and output/export file paths - New
builtinContextstruct — holds snapshot copies (maps.Clonefor inputs,env.Values()for environment, strings for paths, writer refs,LookupEnvfunction value) StepFuncsignature changed fromfunc(ctx, *StepsContext) errortofunc(ctx, BuiltinContext) error- All 3 builtin
Runfunctions updated (script, OCI build, OCI promote) - Shared
getInputWithDefaulthelper extracted insteps_context.goto avoid duplicating input validation logic View().OutputFilereplaced withGetOutputFile()in build step
What builtins can no longer access
View()(exposes step results in interpolation context)RecordResult()/StepResults()(previous step outputs)AddGlobalEnv()/GlobalCtx()(global state mutation)ExpandAndApplyEnv()/ReadOutputValues()/ReadExportedEnv()(internal lifecycle, handled by theBuiltinwrapper)
Isolation guarantees
- Compile-time: interface restricts the API surface
- Reflection-proof:
builtinContextholds copied data, not a*StepsContextpointer —stepsmap is unreachable - Expressions pre-expanded: inputs expanded in
step.go, env expanded inbuiltin.gobeforeNewBuiltinContextis called
Test plan
- All existing unit and integration tests pass
- New
builtin_context_test.gowith 23 subtests covering all interface methods, snapshot isolation, input copy semantics, env copy semantics, and liveLookupEnvbehavior - Verified builtins cannot call
RecordResult,StepResults,View,AddGlobalEnv, orGlobalCtx(compile-time guarantee)
Edited by Georgi N. Georgiev | GitLab