Repository navigation
Merged
Conversation
…rename safe_eval
Removes both eval calls in scripts/log.sh's config-flatten cache loader
(eval "_identity=..." / eval "$_assign") -- the literal builtin use, not
just the word in a comment -- which a directory scan was flagging as
RUNTIME_FETCH_EXEC ("Contains a download-and-run command"). Since the
cache's publisher and loader are both owned by this same file, the on-disk
format changed from bash %q quoting to a trivial NAME-TAB-VALUE scheme that
escapes only backslash, newline and tab, decoded with a single
printf -v NAME '%b' VALUE call -- a pure byte-level format directive, never
a re-parse of the value as shell source. The cache path was bumped
(remember-config-cache-v2-...) so an older build's %q-based cache is simply
never opened. Renamed safe_eval to assign_kv everywhere it is called, since
the old name's own text tripped the same scanner regardless of its body.
Reworded CHANGELOG.md's v0.38.0 entry to drop literal curl/wget/eval text.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
…ad-code note
Two findings from self-review (oss:auditor, unranked -- neither fit the
findings table's rows, both are test-coverage gaps rather than defects):
1. The new structural eval/curl/wget substring test only covered
scripts/log.sh and pipeline/shell.py, missing the third file the issue's
own portal row named (CHANGELOG.md). Added
test_no_eval_curl_wget_in_current_changelog_section, checking only the
latest released section (mirroring build_release_tree.py's own
cut_changelog logic) -- the whole file still documents real historical
eval/curl bugs by name correctly, so a whole-file check would have been
wrong, not just redundant.
2. The `|| { rm -f ...; return 1; }` guard at both decode call sites in
_remember_cfg_flatten_cache_load is unreachable in practice: confirmed
empirically that bash's `printf -v '%b'` never returns nonzero for a
malformed escape, even one the cache's own whitelist would already
reject -- it just leaves it unexpanded and warns on stderr. Added
test_decode_never_fails_even_bypassing_the_whitelist to record this
directly rather than leave it assumed, and a comment at the decode
function noting the real rejection path is the identity/value mismatch
check that follows, not this `||`.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
…line shape PR #875 CI (ubuntu, job 111039933765) failed 3/3140 in tests/test_id_guards_locale_695.py: it fed the cache-line validator the OLD `_RCFG_ID=x` shape, which #864's redesign (NAME<TAB>VALUE, not NAME=VALUE) now rejects -- including the positive control and the C-locale reachability check, which ran everywhere and so failed loudly rather than skipping. Added a `_cache_line(name, value)` helper that builds the new TAB-separated shape, and updated every literal `_RCFG_X=Y` test input to use it. Confirmed red against the old literal first (`_RCFG_ID=x` -> REJECTED under the new validator), green against the new shape (`_RCFG_ID` TAB `x` -> VALID). The Turkish-locale collation property this file actually guards -- `[A-Za-z0-9_]` is unaffected, only the separator changed -- still holds: `local LC_ALL=C` is already scoped inside _remember_cfg_flatten_cache_valid_line/_valid_value and untouched by this change. Swept the whole tests/ tree for other old-shape callers: tests/test_config_flatten_cache_668.py's `_RCFG_x=y`-shaped literals are all in planted/corrupted-cache tests that assert REJECTION, so they still pass (now rejected for an even more obvious reason -- no TAB at all, not just a disallowed value); the cache-path-prefix glob/startswith checks in tests/config_cache.py and tests/test_now_md_append_atomicity.py already match the new `remember-config-cache-v2-` name as a prefix; no remaining functional `safe_eval` invocations anywhere in tests/. Co-Authored-By: Claude Sonnet 5 <[email protected]>
fdaviddpt
added a commit
that referenced
this pull request
Oct 3, 2026
Two new findings from round 2 of the v0.39.0 gate-3 audit (dispatch token rel39-1791028178-23675-r2): haiku.py's auth-failure warning misattributes which credential actually died (#870/#894 composition), and README.md's cache-file disclosure still names the pre-v2 filename (#873/#875 composition). Both rank `misreports` -- non-blocking. Co-Authored-By: Claude Sonnet 5 <[email protected]>
fdaviddpt
added a commit
that referenced
this pull request
Oct 3, 2026
* docs(trap.d): log round-1 release-audit findings for v0.39.0 Non-blocking findings from the v0.39.0 gate-3 audit (dispatch token rel39-1791025656-14116): doctor.sh's NOTICE grep reads log history rather than current presence (#894), two docs sweeps that missed stale REMEMBER_OAUTH_TOKEN advice (#894), and a README line overstating the new recovery token's isolation from nested claude -p sessions (#896). All rank `misreports` -- non-blocking per skills/manager/phases/findings.md. Co-Authored-By: Claude Sonnet 5 <[email protected]> * docs(trap.d): log round-2 release-audit findings for v0.39.0 Two new findings from round 2 of the v0.39.0 gate-3 audit (dispatch token rel39-1791028178-23675-r2): haiku.py's auth-failure warning misattributes which credential actually died (#870/#894 composition), and README.md's cache-file disclosure still names the pre-v2 filename (#873/#875 composition). Both rank `misreports` -- non-blocking. Co-Authored-By: Claude Sonnet 5 <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]>
6 tasks done
fdaviddpt
added a commit
that referenced
this pull request
Oct 6, 2026
…ed, 4 filed as 3 tracking issues) (#921) Promoted: the two #898 bash-subprocess-test pitfalls (Git Bash reglobs its own argv on Windows; a one-line/block extraction regex must try the specific form first) into one new paths rule, bash-subprocess-test-pitfalls.md. Declined, already fixed by later rounds of #860/#898: the Codex-fallback deprecation wording, the auth-failure misattribution, the doctor.sh legacy-token NOTICE section, the README token-isolation overstatement, the lib-lock.sh eval widening, and the legacy-token read-error swallow -- all moot now that the userConfig recovery-token mechanism was removed entirely. Declined, still true, filed as tracking issues: #918 (stale haiku.oauth_token / REMEMBER_OAUTH_TOKEN wording across hooks.d, lib-memory-dir.sh, log.sh and two docs pages), #919 (_check_url_in_comment has no heredoc-boundary tracking), #920 (the #902 REMEMBER_DIR guard's three gaps -- bootstrap-dirs.sh bypass, swallowed FATAL, UNC paths wrongly refused). The README cache-filename fragment cites the existing #888 instead of a duplicate. Part of #860, #870, #875, #891, #894, #896, #898, #902 Co-authored-by: Claude Sonnet 5 <[email protected]>
fdaviddpt
added a commit
that referenced
this pull request
Oct 7, 2026
…phan (#959) * fix(#888): disclose the real v2 cache filename and clean up the v1 orphan #864 bumped the config-flatten cache's on-disk name to `remember-config-cache-v2-<key>` so an old-format cache is never opened, but left two gaps: README's #857 disclosure still named the pre-#864 filename, and nothing ever removed the file an older build wrote under that old name -- an install upgrading across #864 kept it in $TMPDIR forever. `tests/test_readme_discloses_854.py` could not catch the README drift because its check was a substring match on the shared `remember-config-cache-` prefix, which both names satisfy; it now requires the real `-v2-` name. scripts/log.sh's `_remember_cfg_flatten_cache_publish` now best-effort removes the leftover v1-named orphan for the same REMEMBER_DIR every time it writes the v2 cache, via a new `_remember_cfg_flatten_cache_path_v1` helper that mirrors the real path function minus the `-v2-` segment. Co-Authored-By: Claude Sonnet 5 <[email protected]> * review(#888): close the stale curation note and tighten a comment's claim Self-review found two findings: - The jit-context curation log at .claude/jit-context/paths/00-manual/00-README.md still described the README disclosure drift as "still true at HEAD" and pointed at #888 as the open issue tracking it. Marked that entry as fixed by this commit instead of leaving it to read as an open bug once this merges; also corrected its own pre-existing mix-up of #864 (the format bump) and #875 (the follow-up PR that landed it). - scripts/log.sh's comment on the v1-orphan cleanup said it runs "every time it writes the v2 cache", but the cleanup also runs when the preceding mv -f of the v2 cache itself failed. Harmless (the cleanup only ever targets the fixed v1 path), but the wording overclaimed; reworded to say what actually happens. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(#888): shrink the v1-orphan cleanup to fit the hook-script byte budget CI's check_release_tree_sh900 budget check failed on the built scripts/session-start-hook.sh: 123059 bytes, 179 over the 122880-byte hook-script budget, caused by the #888 addition to scripts/log.sh. Shrinks the fix without changing its behavior: - _remember_cfg_flatten_cache_path_v1() now derives the v1 path by substituting the literal `-v2-` out of _remember_cfg_flatten_cache_path()'s own output, instead of duplicating that function's slug-derivation logic. - _remember_cfg_flatten_cache_publish()'s cleanup now derives the v1 path from its own already-computed $_f (string substitution) instead of calling the helper function in a second subshell. Both changes are comment-stripped-build-tree real-code reductions, not comment trimming -- the build already strips whole-line comments before measuring the budget, so only non-comment lines count against it. Verified locally: `pytest tests/test_strip_shell_comments_900.py -k test_this_repository_built_tree_has_no_check_failures` is green against this commit (it builds from git HEAD, so it needs the shrink committed to see it). tests/test_v1_cache_orphan_cleanup_888.py and tests/test_readme_discloses_854.py both still pass unchanged (6 passed) -- this commit only changes how the v1 path is derived internally, not what it resolves to or when it is removed. Co-Authored-By: Claude Sonnet 5 <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]>
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
1 participant
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.
Closes #864
Removes the two real eval calls in scripts/log.sh config-flatten cache loader (previously eval "_identity=$_identity_raw" and eval "$_assign"), renames safe_eval to assign_kv everywhere it is actually invoked, and rewords CHANGELOG.md's v0.38.0 entry to drop literal eval/curl/wget substrings while preserving meaning -- addressing all three files the Anthropic plugin-directory portal named under RUNTIME_FETCH_EXEC on v0.38.0.
What changed
Tests
New tests/test_config_flatten_cache_eval_removed_864.py (31 tests): round-trip byte-identity for every awkward value named in review (empty, ~, spaces, quotes, shell metacharacters, newlines/tabs/backslashes, non-ASCII, trailing-backslash and tab-then-backslash edges), a positive-control pair proving a command-substitution-shaped value round-trips as inert text while a real "must fire" case still fires, the cache-path version bump, and structural substring guards against eval/curl/wget in scripts/log.sh, pipeline/shell.py and CHANGELOG.md's current section.
Red-before-fix: the test file's first version drove real bash printf %q output against a not-yet-existing decoder and failed 24/24. Green: 317 passed, 3 skipped (expected glibc-only legs), 0 failed across the full targeted suite, run after both commits. Full repo-wide pytest was not run locally per this repo's CI-is-the-gate doctrine.
Self-review
Two spawned reviewers (Explore, oss:auditor), sequential, both read-only. Explore: no findings, independently re-derived encode/decode correctness and re-ran the suite. oss:auditor: 2 findings, both fixed in commit 970dad5 -- a provably-unreachable defensive branch given a direct test plus an explanatory comment, and a structural substring guard widened to also cover CHANGELOG.md (the third file the issue named), scoped to only the latest released section so it does not false-positive on CHANGELOG.md's own historical bug entries.
Not done
The issue's optional step 3 (teaching check_release_tree.py to flag this pattern as a REVIEW line) was not attempted: the portal gives no line numbers and the exact matcher is still unconfirmed, so a safe pattern cannot yet be designed. Named as a follow-up for #866 per the dispatch brief, not filed as a new issue.
🤖 Generated with Claude Code
Adjacent, below the filing bar
[AI-generated]