Test - #17
Conversation
Rename NpcActionSystem.cs to NpcActionWork.cs as part of event handling refactor to improve code organization and maintainability.
…ssage handling - Replace hardcoded BepInPlugin values with BattleLuckPluginInfo constants for better maintainability. - Fix NpcControlMode to NpcBehaviorMode in leash check to align with API changes. - Overhaul ChatMessageSystem patch: remove unused AI service import, replace `GetChannel` with `IsInAiChannel`, add try-catch, and clarify entity destruction logic.
- Create GitHub Actions workflow for BattleLuck build on Windows-latest runner - Configure .NET 6 SDK installation with caching for dependencies - Add steps for checkout, restore, build release, and whitespace error check feat(chat): implement AI channel commands for chat management - Add commands to show, enter, leave, toggle, and display current AI chat channel - Refactor to streamline channel state switching and user notifications - Improve reply messages explaining channel behavior and status feat(chat): add AI channel message service for formatted chat delivery - Implement message broadcasting with color-coded AI channel text - Sanitize input to prevent formatting injection and ensure safe display - Support chunking of long messages to adhere to length limits - Provide methods to send status and error messages privately or broadcast refactor(runtime): integrate AI channel cleanup with player lifecycle events - Subscribe to GameEvents.OnPlayerLeft to remove AI channel membership on disconnect - Clear AI channel state on ProjectMEventRouter shutdown to avoid stale data - Remove redundant player-state tracking for AI channel management
Introduce BacktraceConfig model classes for error reporting service configuration including database and reporting settings. Add comprehensive setup guide in BACKTRACE_SETUP.md documenting integration with Unity log handler, automatic error capture, and manual error reporting capabilities.
…vices - Introduce ServerEventPlatform for canonical event ledger, scoring, and results platform - Add PlayerDirectoryService with deterministic normalization and rename validation - Implement IErrorReporter interface with BacktraceHttpErrorReporter and NoOpErrorReporter - Report errors on server tick and core initialization failures - Initialize new services during BattleLuck plugin loading - Dispose of added services and error reporter on unload - Update GitHub Actions workflow to use .NET 8.0, run tests, and package server artifact - Remove AI runtime, commands.txt, docker-compose.ai.yml, and related files no longer used - Add new unit tests for error reporting, player directory, kill attribution, and server event platform - Skip some tests dependent on Unity.Entities binaries for V Rising dedicated-server environment - Clean up embedded resources in BattleLuck.csproj removing helper scripts/docs embedding - Add dist/BattleLuck/BouncyCastle.Cryptography.dll to .gitignore to exclude build output files
- Introduce `UnifiedPlannerService` with `DeveloperBridgeStrategy` and `CombatDrillStrategy`. - Initialize, configure, and dispose of the new planning service in `BattleLuckPlugin`. - Include `render-prefabs.json` and `prefab-actions.json` data files in the project. - Register planning-related services in the core lifecycle.
- Add adaptive NPC behavior controllers (movement, combat, observation, pattern recognition) - Add adaptive event orchestrator for AI-driven NPC spawning and behavior - Add combat drill system for structured NPC training scenarios - Add event catalog system for managing NPC spawn configurations - Add participant analysis for dynamic difficulty adjustment - Add reward limiter for controlled item distribution - Update NpcControlService with GetEntry method - Add comprehensive adaptive event models Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… fix, placement radius contract, bounded work queue
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| function adminSecurityGuard(req: Request, res: Response, next: NextFunction) { | ||
| const isLocal = | ||
| req.hostname === "localhost" || | ||
| req.ip === "127.0.0.1" || | ||
| req.ip === "::1" || | ||
| req.ip === "::ffff:127.0.0.1"; | ||
|
|
||
| const token = req.headers["x-admin-token"]; | ||
| const configuredToken = process.env.ADMIN_TOKEN; | ||
|
|
||
| if (isLocal || !configuredToken || token === configuredToken) { | ||
| return next(); | ||
| } | ||
|
|
||
| res.status(403).json({ error: "Unauthorized: Admin authorization required." }); |
There was a problem hiding this comment.
🚨 Security: Admin write endpoints unauthenticated when ADMIN_TOKEN unset
adminSecurityGuard calls next() whenever !configuredToken is true, and the server binds to 0.0.0.0 (all interfaces). If ADMIN_TOKEN is not set in the environment, every mutating endpoint (/api/bepinex/import, /api/map/markers|resources|zones, delete, /api/map/reload) is fully open to any remote host, allowing arbitrary writes/deletes to the server's BepInEx config files. Default to deny when no token is configured (and/or bind to 127.0.0.1), rather than allowing open access.
Never grant access when no token is configured; only allow loopback or a matching configured token.:
function adminSecurityGuard(req, res, next) {
const isLocal = req.ip === "127.0.0.1" || req.ip === "::1" || req.ip === "::ffff:127.0.0.1";
const token = req.headers["x-admin-token"];
const configuredToken = process.env.ADMIN_TOKEN;
if (isLocal) return next();
if (configuredToken && token === configuredToken) return next();
res.status(403).json({ error: "Unauthorized: Admin authorization required." });
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| // Environment variable or default V Rising Dedicated Server BepInEx path | ||
| const RAW_BEPINEX_ROOT = | ||
| process.env.BEPINEX_ROOT || | ||
| "C:\\Users\\ahmad\\OneDrive\\Desktop\\DedicatedServerLauncher\\VRisingServer\\BepInEx"; | ||
|
|
||
| // Determine effective BEPINEX_ROOT path (fallback to local mock directory if physical Windows path does not exist) | ||
| let BEPINEX_ROOT = path.resolve(RAW_BEPINEX_ROOT); | ||
| if (!fs.existsSync(BEPINEX_ROOT)) { | ||
| BEPINEX_ROOT = path.resolve(process.cwd(), "mock_bepinex", "BepInEx"); | ||
| } |
There was a problem hiding this comment.
⚠️ Security: Hardcoded personal absolute path and username committed
A developer's personal Windows path including the username ahmad and OneDrive layout is hardcoded as the default BEPINEX_ROOT and shown in the UI/export payload. This leaks personal environment details and is not a sensible default for any other deployment. Replace with a neutral relative default (e.g. the mock_bepinex fallback) and require BEPINEX_ROOT/serverStatus.activeRoot to be supplied by config.
Was this helpful? React with 👍 / 👎
| function resolveSafePath(userRelativePath: string): string { | ||
| const resolved = path.resolve(BEPINEX_ROOT, userRelativePath); | ||
| if (!resolved.startsWith(BEPINEX_ROOT)) { | ||
| throw new Error("Security Violation: Access outside BepInEx root is forbidden."); | ||
| } | ||
| return resolved; | ||
| } |
There was a problem hiding this comment.
💡 Quality: resolveSafePath is dead code with a prefix-match flaw
resolveSafePath is defined but never called; all file routes use fixed paths. Beyond being dead code, its containment check resolved.startsWith(BEPINEX_ROOT) is the classic prefix bug (e.g. a sibling dir BepInEx-evil passes). If it is ever wired up to user-controlled paths as intended, it would not actually prevent traversal. Either remove it, or fix it to compare against BEPINEX_ROOT + path.sep / use path.relative.
Fix:
function resolveSafePath(userRelativePath: string): string {
const resolved = path.resolve(BEPINEX_ROOT, userRelativePath);
const rel = path.relative(BEPINEX_ROOT, resolved);
if (rel.startsWith("..") || path.isAbsolute(rel)) {
throw new Error("Security Violation: Access outside BepInEx root is forbidden.");
}
return resolved;
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| const exists = | ||
| placedResourceNodes.some(r => r.id === id) || | ||
| customMarkers.some(m => m.id === id) || | ||
| patrolRoutes.some(p => p.id === id); |
There was a problem hiding this comment.
💡 Bug: Import duplicate-detection ignores containers and radii
In handleGeneratePreview, the exists check only scans placedResourceNodes, customMarkers, and patrolRoutes, omitting placedContainers and placedRadii. Items whose id already exists as a container/radius are counted as added rather than changed/skipped, making the preview counts misleading. Include all five collections in the existence check.
Was this helpful? React with 👍 / 👎
Code Review 🚫 Blocked 0 resolved / 6 findingsIntroduces an adaptive NPC control system, unified planner service, and natural language AI action routing alongside map dashboard UI components. Blocked by multiple critical and important findings including unauthenticated admin write endpoints, hardcoded personal absolute paths, and unrelated Bible translation assets bundled in the pull request. 🚨 Security: Admin write endpoints unauthenticated when ADMIN_TOKEN unset📄 website/server.ts:91-105 📄 website/server.ts:515 📄 PJ/APP/server.ts
Never grant access when no token is configured; only allow loopback or a matching configured token.
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
1 participant
Summary by Gitar
NaturalLanguageActionRouterandActionProposalStorefor imperative.airequests with validation and admin approval gatesRuntimeEffectActionServiceand catalog handling for temporary request-scoped status effects and world VfxBepInExDataModal.tsx) for JSON map data import, validation, and exportThis will update automatically on new commits.