judge: add message to inform sandbox upgrade #1111
Conversation
WalkthroughThis change adds a new internal flag Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/hydrojudge/src/sandbox.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/hydrojudge/src/sandbox.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/hydrojudge/src/sandbox.ts (2)
222-222: LGTM!Variable declaration follows the established pattern in the function and is appropriately typed.
229-229: LGTM!The default value of
falseensures that sandboxes without this config property will be flagged as vulnerable, which is appropriate for a security-critical feature (fail-safe approach).
| if (!fixSymlinkEscape) { | ||
| reportError('Your sandbox version is vulnerable of symlink escape issue, please upgrade!') | ||
| } | ||
| return true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, run the provided script to find versionCheck usages
rg -nP -A5 -B2 '\bversionCheck\s*\(' --type=tsRepository: hydro-dev/Hydro
Length of output: 1965
🏁 Script executed:
# Also, let's examine the sandbox.ts file to see the actual code at lines 231-250
wc -l packages/hydrojudge/src/sandbox.tsRepository: hydro-dev/Hydro
Length of output: 97
🏁 Script executed:
# Read the relevant section of sandbox.ts
sed -n '225,260p' packages/hydrojudge/src/sandbox.tsRepository: hydro-dev/Hydro
Length of output: 1491
Return false when symlink escape vulnerability is detected to halt execution safely.
The function reports a critical security error when fixSymlinkEscape is false but still returns true, inconsistent with lines 231-233 where false is returned after reporting other errors. In daemon.ts:49, the caller checks this return value and exits if false; returning true despite the vulnerability allows the system to proceed unsafely.
🤖 Prompt for AI Agents
In packages/hydrojudge/src/sandbox.ts around lines 247 to 250, the code reports
a symlink escape vulnerability when fixSymlinkEscape is false but still returns
true; change the control flow to return false after calling reportError so
execution halts consistently with other error cases (lines 231–233) and the
caller in daemon.ts can safely exit; ensure reportError is called first and then
return false.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.