fix(#716): item 3 — document headless getenv path; refactor legacy guard

[jsavin]

Summary

Closes item 3 of #716 with two findings and a small refactor:

1. The audit pointed at the wrong file. Common/source/shellsysverbs.c::getenvironmentvariablefunc is the legacy (full Mac app) impl; it is not linked into the headless frontier-cli build. The actual headless verb dispatch goes to tests/headless_sys_verbs.c::sysv_getenvironmentvariable.

2. The headless path has no truncation surface by construction. It uses newfilledhandle + setheapvalue to return a heap-allocated string handle. There is no bigstring on this path, so the >255-byte concern raised in the issue does not apply. A regression test now locks this in.

3. Legacy file got a small post-copyctopstring: silent truncation + buffer overflow risk on >255-byte input #707 refactor. The legacy guard worked, but it predated the post-copyctopstring: silent truncation + buffer overflow risk on >255-byte input #707 boolean-return convention. Collapsed 12 lines to 8, matching frontier-cli/window_registry.c:116 and PR fix(#716): parseerror signature now const char *; drop Pascal-string laundering #718's langerror.c:163 fix. Same observable behavior, same user-visible langerrormessage.

Changes

• tests/headless_sys_verbs.c — expand the @IMPLEMENTED comment to document the heap-handle design intent, explicitly call out "no bigstring truncation surface here", and mark the path as "do not add a bigstring length guard" with a back-reference to copyctopstring audit tail: 6 sibling defects surfaced during #712 (bucket) #716 item 3
• Common/source/shellsysverbs.c — refactor the legacy guard from strlen + manual >255 check + unguarded copyctopstring to the canonical post-copyctopstring: silent truncation + buffer overflow risk on >255-byte input #707 form (copyctopstring's boolean return)
• tests/integration/sys_getenv_long_value_test.sh (new) — shell-level integration test asserting:
  • 200-byte env var reads back at length 200 (control)
  • 300-byte env var reads back at length 300 (no truncation)
  • 300-byte env var content preserved verbatim through byte 300
• tests/Makefile — wire test-sys-getenv-long-value into test-all and .PHONY

Test plan

• make build — clean
• ./tools/run_headless_tests.sh — 589/589 pass
• cd tests && make test-sys-getenv-long-value — 3/3 pass
• cd tests && make test-integration — 21 baseline failures match PR test(#715): behavioral coverage for copyctopstring deep-stack defenses #717's stated baseline (html / tcp / startup flakiness, unrelated to env-var code)

Scope

This PR addresses only item 3 of #716. Items 2, 4, 5 remain open and will be addressed in separate PRs:

• 2: strings.c push-after-copyctopstring truncation
• 4: langsqlite.c sqlite3_errmsg truncation
• 5: tcpverbs.c / WinSockNetEvents.c INDIRECT callsites

Item 6 was already closed via PR #717's analysis.

Investigative process

I initially wrote a test that called sys.getenvironmentvariable with a >255-byte env var expecting an error (matching the original issue framing). The test returned the full 300 bytes — which surfaced that the audit target wasn't on the live path. Traced the verb dispatch through the kernel-verbs generator to tests/headless_sys_verbs.c and found the heap-handle design. The test was reframed as a regression guard for the actual headless behavior.

🤖 Generated with Claude Code

[jsavin]

/auto Summary

What went well: The TDD-first habit caught the audit misdirection — writing the test first surfaced that the original target (Common/source/shellsysverbs.c) wasn't on the live verb-dispatch path, and led to discovering that the real headless verb (tests/headless_sys_verbs.c) uses heap handles by design. Rescoping the PR from "fix a bug" to "document the design + small legacy cleanup + lock-in regression test" was straightforward once the layout was understood.

What could improve: The original #716 audit greps for copyctopstring in Common/source/ but doesn't filter for live reachability in the headless build. The legacy Common/source/shellsysverbs.c is one of several files in that tree that are dead code in the headless target. A follow-up improvement to the audit pattern would be to cross-check each callsite against the kernel_verbs_init.c generator output before declaring a defect — for the remaining #716 items (2, 4, 5), this verification should happen first.

Recommendations: For items 4 (langsqlite errmsg) and 5 (tcpverbs INDIRECT), verify live reachability through generated/kernel_verbs_init.c before any code change. Item 2 (strings.c:1315 - string.dateString helpers) is harder to map at a glance and may warrant a dedicated reachability audit before scoping the fix.

🤖 Generated with Claude Code /auto