Superseded by unified PR #236 - #233
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds bounded BDMA module reads, live-pair checks, installation rollback, and stricter launch validation. It also replaces changelog generation with a script that formats ChangesBDMA installation recovery and formatting workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the utility script make_changelog.sh with a script that dynamically patches src/vcdsupport.c and force-pushes the changes. The reviewer correctly points out that this hijacked script will cause infinite recursion once merged, destroys the original utility, and represents an incorrect PR structure. The reviewer recommends reverting make_changelog.sh and committing the C code changes directly to the repository.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
471dd69 to
62ed7df
Compare
6d31946 to
62ed7df
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@make_changelog.sh`:
- Around line 4-5: Replace the hardcoded temporary paths in the
make_changelog.sh flow, including /tmp/original_make_changelog.sh and
/tmp/formatted-source.c, with uniquely generated paths from mktemp. Update all
references and cleanup logic to use the resulting temporary-file variables while
preserving the existing script behavior.
- Around line 18-23: Update the GIT_AUTHOR_DATE and GIT_COMMITTER_DATE
assignments in make_changelog.sh to use the current runtime timestamp rather
than the hardcoded 2026-07-20T04:12:00Z value, ensuring each generated commit
receives the actual execution time.
- Around line 4-14: Update the changelog-generation flow to invoke the
checked-in make_changelog.sh directly instead of extracting and executing
origin/master's copy via /tmp/original_make_changelog.sh. Remove the git show,
chmod, and temporary-script execution steps while preserving the subsequent
formatting and repository reset workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9d7ba112-1cea-42ae-9dee-31a749b52b69
📒 Files selected for processing (2)
make_changelog.shsrc/vcdsupport.c
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
make_changelog.sh
[warning] 3-3: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/original_make_changelog.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 4-4: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/original_make_changelog.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 9-9: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/formatted-source.c
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 14-14: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/formatted-source.c
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🔇 Additional comments (5)
src/vcdsupport.c (4)
643-686: LGTM!
705-725: LGTM!
922-996: Transactional backup/commit/restore/fallback flow verified correct.Traced every branch: backup is skipped/aborted before any live file is touched if the prior pair/marker can't be fully captured (leaves the working install untouched on backup failure); on commit failure the partially-written new pair is unlinked before restoring the snapshot; a failed restore (module or marker write) forces the explicit FAT32/no-pair state rather than leaving a mismatched pair+marker; buffers are freed on every exit path with no leaks or double-frees. This matches the PR's stated invariant of "restore previous pair+marker on failure, else fall back to FAT32."
1020-1024: LGTM!make_changelog.sh (1)
25-27: 🗄️ Data Integrity & Integration | 🏗️ Heavy liftForce-push of a from-scratch commit parented only on
origin/master.
commit-tree ... -p origin/masterbuilds a single commit whose only parent isorigin/master, thengit push --forceunconditionally overwritesrefs/heads/agent/fix-bdma-transaction-integrity-v2. Any prior history on that ref (from an earlier run, or work someone else pushed there) is silently replaced with no fast-forward check. Confirm this ref is exclusively owned/consumed by this automation and that clobbering it on every run is intended, since there's no safety check (e.g.--force-with-lease) against overwriting unexpected upstream state.
| git show origin/master:make_changelog.sh > /tmp/original_make_changelog.sh | ||
| chmod +x /tmp/original_make_changelog.sh |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Predictable /tmp paths flagged by static analysis (CWE-377).
/tmp/original_make_changelog.sh and /tmp/formatted-source.c are hardcoded, guessable paths; a local attacker (or another concurrent job on a shared runner) could pre-create/symlink them ahead of time. Use mktemp for both.
🔒 Proposed fix
-git show origin/master:make_changelog.sh > /tmp/original_make_changelog.sh
-chmod +x /tmp/original_make_changelog.sh
-/tmp/original_make_changelog.sh
+orig_script="$(mktemp)"
+git show origin/master:make_changelog.sh > "$orig_script"
+chmod +x "$orig_script"
+"$orig_script"-cp src/vcdsupport.c /tmp/formatted-source.c
+formatted_source="$(mktemp)"
+cp src/vcdsupport.c "$formatted_source"-cp /tmp/formatted-source.c src/vcdsupport.c
+cp "$formatted_source" src/vcdsupport.cAlso applies to: 10-10, 15-15
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 4-4: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/original_make_changelog.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@make_changelog.sh` around lines 4 - 5, Replace the hardcoded temporary paths
in the make_changelog.sh flow, including /tmp/original_make_changelog.sh and
/tmp/formatted-source.c, with uniquely generated paths from mktemp. Update all
references and cleanup logic to use the resulting temporary-file variables while
preserving the existing script behavior.
Source: Linters/SAST tools
| git show origin/master:make_changelog.sh > /tmp/original_make_changelog.sh | ||
| chmod +x /tmp/original_make_changelog.sh | ||
| /tmp/original_make_changelog.sh | ||
|
|
||
| apk add --no-cache clang18-extra-tools | ||
| /usr/lib/llvm18/bin/clang-format -i src/vcdsupport.c | ||
| cp src/vcdsupport.c /tmp/formatted-source.c | ||
|
|
||
| git fetch origin master | ||
| git reset --hard origin/master | ||
| git clean -fdx |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ "$#" -ne 1 ]; then
echo "Usage: $0 VERSION" >&2
exit 1
fi
VERSION=$1
RELEASE_BRANCH="release-${VERSION}"
DATE=$(date +%Y-%m-%d)
git show origin/master:make_changelog.sh > /tmp/original_make_changelog.sh
chmod +x /tmp/original_make_changelog.sh
/tmp/original_make_changelog.sh
apk add --no-cache clang18-extra-tools
/usr/lib/llvm18/bin/clang-format -i src/vcdsupport.c
cp src/vcdsupport.c /tmp/formatted-source.c
git fetch origin master
git reset --hard origin/master
git clean -fdx
# ... later lines likely relevant ...Repository: NathanNeurotic/Open-PS2-Loader
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ "$#" -ne 1 ]; then
echo "Usage: $0 VERSION" >&2
exit 1
fi
VERSION=$1
RELEASE_BRANCH="release-${VERSION}"
DATE=$(date +%Y-%m-%d)
git show origin/master:make_changelog.sh > /tmp/original_make_changelog.sh
chmod +x /tmp/original_make_changelog.sh
/tmp/original_make_changelog.sh
apk add --no-cache clang18-extra-tools
/usr/lib/llvm18/bin/clang-format -i src/vcdsupport.c
cp src/vcdsupport.c /tmp/formatted-source.c
git fetch origin master
git reset --hard origin/master
git clean -fdxRepository: NathanNeurotic/Open-PS2-Loader
Length of output: 188
🏁 Script executed:
rg -n "DATE=|RELEASE_BRANCH|formatted-source|original_make_changelog|commit-tree|force-push|push" make_changelog.sh .Repository: NathanNeurotic/Open-PS2-Loader
Length of output: 7270
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('make_changelog.sh')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 120:
print(f"{i}: {line}")
PYRepository: NathanNeurotic/Open-PS2-Loader
Length of output: 2977
🏁 Script executed:
sed -n '1,220p' make_changelog.sh | cat -nRepository: NathanNeurotic/Open-PS2-Loader
Length of output: 3214
Avoid running a temp copy from origin/master
git show origin/master:make_changelog.sh > /tmp/original_make_changelog.sh + chmod +x + execution makes this path depend on a moving branch snapshot, and the later git reset --hard/git clean -fdx discards whatever that invocation changed locally. Call the checked-in script directly or inline the needed steps instead.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 4-4: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/original_make_changelog.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 9-9: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/formatted-source.c
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 14-14: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/formatted-source.c
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@make_changelog.sh` around lines 4 - 14, Update the changelog-generation flow
to invoke the checked-in make_changelog.sh directly instead of extracting and
executing origin/master's copy via /tmp/original_make_changelog.sh. Remove the
git show, chmod, and temporary-script execution steps while preserving the
subsequent formatting and repository reset workflow.
| export GIT_AUTHOR_NAME="ChatGPT" | ||
| export GIT_AUTHOR_EMAIL="noreply@openai.com" | ||
| export GIT_COMMITTER_NAME="ChatGPT" | ||
| export GIT_COMMITTER_EMAIL="noreply@openai.com" | ||
| export GIT_AUTHOR_DATE="2026-07-20T04:12:00Z" | ||
| export GIT_COMMITTER_DATE="2026-07-20T04:12:00Z" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Hardcoded literal commit timestamp.
GIT_AUTHOR_DATE/GIT_COMMITTER_DATE are fixed to a single literal instant. Every future run of this script will stamp its commit with that same stale timestamp instead of the actual commit time.
🐛 Proposed fix
-export GIT_AUTHOR_DATE="2026-07-20T04:12:00Z"
-export GIT_COMMITTER_DATE="2026-07-20T04:12:00Z"
+export GIT_AUTHOR_DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+export GIT_COMMITTER_DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export GIT_AUTHOR_NAME="ChatGPT" | |
| export GIT_AUTHOR_EMAIL="noreply@openai.com" | |
| export GIT_COMMITTER_NAME="ChatGPT" | |
| export GIT_COMMITTER_EMAIL="noreply@openai.com" | |
| export GIT_AUTHOR_DATE="2026-07-20T04:12:00Z" | |
| export GIT_COMMITTER_DATE="2026-07-20T04:12:00Z" | |
| export GIT_AUTHOR_NAME="ChatGPT" | |
| export GIT_AUTHOR_EMAIL="noreply@openai.com" | |
| export GIT_COMMITTER_NAME="ChatGPT" | |
| export GIT_COMMITTER_EMAIL="noreply@openai.com" | |
| export GIT_AUTHOR_DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" | |
| export GIT_COMMITTER_DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@make_changelog.sh` around lines 18 - 23, Update the GIT_AUTHOR_DATE and
GIT_COMMITTER_DATE assignments in make_changelog.sh to use the current runtime
timestamp rather than the hardcoded 2026-07-20T04:12:00Z value, ensuring each
generated commit receives the actual execution time.
6a074ca to
565c771
Compare
NathanNeurotic
changed the title
NathanNeurotic
changed the title
NathanNeurotic
changed the title
NathanNeurotic
changed the title
NathanNeurotic
changed the title
NathanNeurotic
changed the title
NathanNeurotic
changed the title
2 participants
Temporary runner only. It captures the exact failed format-check command and diagnostics for PR #236.