fix: SEC-01 eliminate shell-variable injection in gc-scheduled.sh#60
Closed
majiayu000 wants to merge 3 commits intomainfrom
Closed
fix: SEC-01 eliminate shell-variable injection in gc-scheduled.sh#60majiayu000 wants to merge 3 commits intomainfrom
majiayu000 wants to merge 3 commits intomainfrom
Conversation
…hon blocks
Replace all three python3 -c "...${VAR}..." invocations with
heredoc (<<'PYEOF') style so shell never expands variables inside
Python source code.
- Block 1 (metrics loop): pass ${mf} and ${CUTOFF} via env vars
METRICS_FILE / CUTOFF; Python reads via os.environ; tracks
original line count internally instead of interpolating ${BEFORE}.
- Block 2 (learn-digest): pass LOG_DIR and VIBEGUARD_DIR via env;
fix output.split('\\n') → split('\n') which was only correct by
accident inside a double-quoted -c string.
- Block 3 (reflection): pass LOG_DIR and REFLECTION_FILE via env.
Closes #56
Signed-off-by: majiayu000 <1835304752@qq.com>
…ator docs/how/memory-files.md:9 references `project/.claude/CLAUDE.md` as an example path (not a real repo file). Add it to the allowlist to fix CI. Signed-off-by: majiayu000 <1835304752@qq.com>
Owner
Author
|
/gemini review |
…edoc Python blocks
Backslash escapes inside f-string {} expressions are a SyntaxError in Python.
In a <<'PYEOF' heredoc the shell performs no unescaping, so \" reached Python
as a literal backslash+quote and caused both the learn-digest and reflection
phases to abort silently. Replace \"...\" with "..." inside the four affected
f-string expressions (valid because the outer f-strings use single quotes).
Owner
Author
|
Superseded by PR #58 (already merged) which fixes the same SEC-01 issue using GC env var prefix approach. |
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.
Summary
python3 -c "...${VAR}..."invocations with<<'PYEOF'heredocs so the shell never expands variables inside Python sourceLOG_DIR,VIBEGUARD_DIR,REFLECTION_FILE,mf, andCUTOFFvia the command's environment (VAR=value python3 -) and read them withos.environ[...]${BEFORE}interpolationoutput.split('\\n')→output.split('\n')(was only correct by accident in the-c "..."context)Root cause
${LOG_DIR}derives fromVIBEGUARD_LOG_DIRenv var (line 12). A value containing a single-quote (e.g./tmp/x'; import os; os.system('id') #) breaks the embedded Python syntax and enables arbitrary code execution when the scheduled GC job runs under systemd/launchd.Test plan
bash -n scripts/gc/gc-scheduled.shpasses (syntax OK)VIBEGUARD_LOG_DIRto a path containing a single-quote; confirm the script exits cleanly rather than executing injected codetests/continue to passCloses #56