Fix nullptr crash in RecConfigOverrideFromEnvironment with runroot

[brbzull0]

When runroot is active and the PROXY_CONFIG_* environment variables for path records (bin_path, local_state_dir``, logfile_dir, plugin_dir) are not set, RecConfigOverrideFromEnvironment()returnednullptr. This violated its documented contract ("return either the overridden value, or the original value") and caused a std::logic_erro`r crash during records.yaml parsing:

  basic_string: construction from null is not valid

The nullptr was assigned to std::string in SetRecordFromYAMLNode(), which threw the exception. The bug was masked in autests because the framework always sets these env vars.

Fix: return the original config value instead of nullptr when RecConfigOverrideFromRunroot() matches. The record value is then resolved by Layout::relative() at runtime, producing the correct path.

Add a test that unsets the 4 path env vars to exercise the runroot code path, and validates record values via traffic_ctl config get.

[Copilot]

Pull request overview

Fixes a crash in ATS records.yaml parsing when runroot is enabled and certain PROXY_CONFIG_* path environment variables are unset, by ensuring RecConfigOverrideFromEnvironment() never returns nullptr for those records. This keeps the function’s contract intact and prevents exceptions when converting the returned C string into a std::string.

Changes:

• Update RecConfigOverrideFromEnvironment() to return the original record value (instead of nullptr) for runroot-managed path records when no env override exists.
• Add a gold test that unsets the four path env vars to reproduce the previous crash path and validates resulting record values via traffic_ctl.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/records/RecConfigParse.cc	Prevents nullptr from being returned during env override resolution under runroot, avoiding downstream std::string(nullptr) crashes.
tests/gold_tests/records/records_runroot_null_override.test.py	Adds regression coverage for the runroot + missing env-var scenario and verifies both env overrides and path-record behavior.

Comments suppressed due to low confidence (1)

src/records/RecConfigParse.cc:129

• RecConfigOverrideFromEnvironment() now returns value regardless of whether RecConfigOverrideFromRunroot(name) is true or false (when envval is unset), so this else if branch is redundant and adds an unnecessary RecConfigOverrideFromRunroot() call on the hot path while parsing every record. Consider removing the else if entirely (or otherwise making the runroot branch do something meaningfully different).

  if (envval) {
    return envval;
  } else if (RecConfigOverrideFromRunroot(name)) {
    return value;
  }

  return value;

[zwoop]

Why this else if ? you return "value" regardless of the value of RecConfigOverrideFromRunroot(name)), no?

[zwoop]

I think you should cleanup the else if, I think CoPilot said the same thing.

[brbzull0]

I think you should cleanup the else if, I think CoPilot said the same thing.

thanks for pointing this out. I think I may have introduced a side effect bug. I'll ammend and put this back for review.