feat(evaluator): make toJSON(secrets) return secrets object (#100)

It turns out that toJSON(secrets) has been returning "null" this
whole time — same disease as env, steps, needs, and github before
it. The evaluator had no bare-`secrets` branch in
ExpressionContext::resolve, so resolving the identifier fell
through to the catch-all `_ => Null` and toJSON dutifully
serialised that. Fourth in a series after #96/#97/#98/#99.

The fix is the simplest of the bunch. secrets_context is already
a flat &HashMap<String, String> holding exactly the shape real
GHA exposes as the `secrets` context — no prefix strip, no
exclusion list (unlike #99's github arm), no nested outputs
sub-object (unlike steps/needs). Clone the entries into
ExprValue::String and wrap in ExprValue::Object. Done.

Values are returned in plaintext by design. That matches real
GHA — `toJSON(secrets)` there is not auto-redacted either, and
the common `fromJSON(toJSON(secrets))` pipe-through-an-action
pattern depends on the exact original values surviving the
round-trip. Masking stays where it belongs: the log boundary,
via wrkflw_secrets::SecretMasker, already wired up in engine.rs.

Pulling the masker into the evaluator would (a) break the
fromJSON round-trip, (b) diverge from GHA semantics, and (c)
duplicate a concern that already has one correct home. Please
don't do that.

Tests mirror the #96–#99 suites: populated secrets, empty
context, sorted keys, special-character values (quotes,
backslashes, PEM-style newlines), fromJSON(toJSON(secrets))
round-trip, bare-secrets truthiness, a regression guard that the
bare arm doesn't shadow the existing dotted-access arm, and a
plaintext-values test that pins the no-masking-here decision so
any future switch is deliberate rather than silent.

While at it, drop `secrets` from the lingering
`TODO: support other bare contexts` comment. Only `matrix`
remains.
This commit is contained in:
Gokul
2026-04-18 22:20:55 +05:30
committed by GitHub
parent bb55e7f30f
commit e4752efa8e

View File

@@ -453,7 +453,7 @@ impl<'a> ExpressionContext<'a> {
.unwrap_or(ExprValue::Null),
// Bare context names — return the whole context as an Object so that
// `toJSON(env)` (and similar) can serialise it.
// TODO: support other bare contexts: secrets, matrix
// TODO: support other bare contexts: matrix
"steps" if parts.len() == 1 => {
// Collect all step IDs from both outputs and statuses maps.
let mut all_ids: HashSet<&String> = self.step_outputs.keys().collect();
@@ -551,6 +551,23 @@ impl<'a> ExpressionContext<'a> {
}
ExprValue::Object(map)
}
"secrets" if parts.len() == 1 => {
// Wrap `secrets_context` as an Object so `toJSON(secrets)` can
// serialise it. Mirrors real GHA's `secrets` context shape
// (flat `{ name: value }` map).
//
// Values are returned in plaintext by design — same policy as
// `toJSON(github)` for `GITHUB_TOKEN`. Masking is a log-boundary
// concern handled by `wrkflw_secrets::SecretMasker` when wired in
// via `engine.rs`. Do not dump this object to untrusted sinks
// without routing through the masker.
let map = self
.secrets_context
.iter()
.map(|(k, v)| (k.clone(), ExprValue::String(v.clone())))
.collect();
ExprValue::Object(map)
}
_ => ExprValue::Null,
}
}
@@ -2294,6 +2311,143 @@ mod tests {
assert_eq!(deploy_outputs.get("emoji").unwrap(), "\u{1F680}");
}
// -- toJSON(secrets) tests --
/// Helper to build an ExpressionContext with secrets data.
fn make_secrets_ctx(secrets: &HashMap<String, String>) -> ExpressionContext<'_> {
ExpressionContext {
env_context: &EMPTY_ENV,
step_outputs: &EMPTY_STEPS,
matrix_combination: &EMPTY_MATRIX,
step_statuses: &EMPTY_STATUSES,
job_status: "success",
secrets_context: secrets,
needs_context: &EMPTY_NEEDS,
needs_results: &EMPTY_NEEDS_RESULTS,
}
}
#[test]
fn tojson_secrets_returns_object() {
let mut secrets = HashMap::new();
secrets.insert("NPM_TOKEN".to_string(), "abc".to_string());
secrets.insert("DEPLOY_KEY".to_string(), "xyz".to_string());
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("toJSON(secrets)", &ctx).unwrap();
let s = result.to_output_string();
let parsed: serde_json::Value = serde_json::from_str(&s).expect("should be valid JSON");
let obj = parsed.as_object().expect("should be a JSON object");
assert_eq!(obj.get("NPM_TOKEN").unwrap(), "abc");
assert_eq!(obj.get("DEPLOY_KEY").unwrap(), "xyz");
assert_eq!(obj.len(), 2);
}
#[test]
fn tojson_secrets_empty_context() {
let secrets = HashMap::new();
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("toJSON(secrets)", &ctx).unwrap();
let s = result.to_output_string();
let parsed: serde_json::Value = serde_json::from_str(&s).expect("should be valid JSON");
let obj = parsed.as_object().expect("should be a JSON object");
assert!(obj.is_empty(), "should be empty with no secrets: {}", s);
}
#[test]
fn tojson_secrets_sorted_keys() {
let mut secrets = HashMap::new();
secrets.insert("ZEBRA".to_string(), "z".to_string());
secrets.insert("APPLE".to_string(), "a".to_string());
secrets.insert("MANGO".to_string(), "m".to_string());
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("toJSON(secrets)", &ctx).unwrap();
let s = result.to_output_string();
let apple_pos = s.find("APPLE").unwrap();
let mango_pos = s.find("MANGO").unwrap();
let zebra_pos = s.find("ZEBRA").unwrap();
assert!(apple_pos < mango_pos, "APPLE should come before MANGO");
assert!(mango_pos < zebra_pos, "MANGO should come before ZEBRA");
}
#[test]
fn tojson_secrets_preserves_special_characters() {
let mut secrets = HashMap::new();
secrets.insert("QUOTE".to_string(), "he said \"hi\"".to_string());
secrets.insert("BACKSLASH".to_string(), "path\\to\\key".to_string());
secrets.insert(
"NEWLINE".to_string(),
"-----BEGIN-----\nBODY\n-----END-----".to_string(),
);
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("toJSON(secrets)", &ctx).unwrap();
let s = result.to_output_string();
let parsed: serde_json::Value =
serde_json::from_str(&s).expect("should be valid JSON despite special chars");
let obj = parsed.as_object().unwrap();
assert_eq!(obj.get("QUOTE").unwrap(), "he said \"hi\"");
assert_eq!(obj.get("BACKSLASH").unwrap(), "path\\to\\key");
assert_eq!(
obj.get("NEWLINE").unwrap(),
"-----BEGIN-----\nBODY\n-----END-----"
);
}
#[test]
fn fromjson_tojson_secrets_produces_parseable_json() {
// `fromJSON` currently returns the raw JSON text as `ExprValue::String`
// (same pattern as `fromjson_tojson_env_produces_parseable_json`). The
// round-trip must preserve exact values so pipe-through-an-action use
// cases work and so any future switch to value-masking here is a
// deliberate decision.
let mut secrets = HashMap::new();
secrets.insert("NPM_TOKEN".to_string(), "npm_ABC123".to_string());
secrets.insert("DEPLOY_KEY".to_string(), "deploy_XYZ".to_string());
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("fromJSON(toJSON(secrets))", &ctx).unwrap();
let s = result.to_output_string();
let parsed: serde_json::Value = serde_json::from_str(&s).expect("should be valid JSON");
let obj = parsed.as_object().expect("should be a JSON object");
assert_eq!(obj.get("NPM_TOKEN").unwrap(), "npm_ABC123");
assert_eq!(obj.get("DEPLOY_KEY").unwrap(), "deploy_XYZ");
}
#[test]
fn bare_secrets_is_truthy() {
let mut secrets = HashMap::new();
secrets.insert("NPM_TOKEN".to_string(), "abc".to_string());
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("secrets", &ctx).unwrap();
assert!(result.is_truthy());
}
#[test]
fn bare_secrets_does_not_shadow_dotted_access() {
// Regression guard: the bare-`secrets` arm must not shadow the existing
// `secrets.NAME` dotted-access arm.
let mut secrets = HashMap::new();
secrets.insert("NPM_TOKEN".to_string(), "abc".to_string());
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("secrets.NPM_TOKEN", &ctx).unwrap();
assert_eq!(result, ExprValue::String("abc".to_string()));
}
#[test]
fn tojson_secrets_returns_values_in_plaintext() {
// Documents current behavior: secret values surface in plaintext inside
// `toJSON(secrets)`. This matches real GHA; masking lives at the log
// boundary via `wrkflw_secrets::SecretMasker`, not in the evaluator.
// Pin the behavior so any future change (exclude, redact, route through
// a masker at this layer) is a deliberate decision.
let mut secrets = HashMap::new();
secrets.insert("GITHUB_TOKEN".to_string(), "ghs_supersecret".to_string());
let ctx = make_secrets_ctx(&secrets);
let result = evaluate("toJSON(secrets)", &ctx).unwrap();
let s = result.to_output_string();
let parsed: serde_json::Value = serde_json::from_str(&s).expect("should be valid JSON");
let obj = parsed.as_object().unwrap();
assert_eq!(obj.get("GITHUB_TOKEN").unwrap(), "ghs_supersecret");
}
#[test]
fn object_cmp_returns_none() {
// Object comparisons via <, >, <=, >= should all evaluate to false