Create .codeforge/ user customization directory#33
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (42)
📝 WalkthroughWalkthroughThis pull request restructures the project's configuration management from a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant setup.sh
participant setup-migrate-codeforge.sh
participant setup.js
participant .codeforge/config
participant FileManifest
User->>setup.sh: Run setup in new/existing container
activate setup.sh
setup.sh->>setup.sh: Validate CODEFORGE_DIR or fallback to old path
setup.sh->>setup-migrate-codeforge.sh: Call migration script
deactivate setup.sh
activate setup-migrate-codeforge.sh
setup-migrate-codeforge.sh->>setup-migrate-codeforge.sh: Check if .codeforge/ exists (skip if present)
setup-migrate-codeforge.sh->>setup-migrate-codeforge.sh: Create .codeforge/config/rules, scripts, .markers, .checksums
setup-migrate-codeforge.sh->>.devcontainer/config/defaults/: Read legacy config files
setup-migrate-codeforge.sh->>.codeforge/config/: Copy and transform defaults/* to config/*
setup-migrate-codeforge.sh->>FileManifest: Rewrite file-manifest.json (defaults/ → config/)
setup-migrate-codeforge.sh->>.codeforge/scripts/: Copy terminal scripts
setup-migrate-codeforge.sh->>setup-migrate-codeforge.sh: Write migration marker with timestamp
deactivate setup-migrate-codeforge.sh
setup.sh->>setup.js: Call with --install or --force
activate setup.js
setup.js->>setup.js: Compute checksums for .codeforge/ directory
setup.js->>setup.js: Read previous checksums from .checksums/<version>.json
setup.js->>setup.js: Identify changed, added, and preserved files via checksum comparison
setup.js->>.codeforge/: Sync changes to destination with preservation logic
setup.js->>setup.js: Write new checksums to .checksums/<version>.json
alt User runs "codeforge config apply"
User->>setup.js: Run config apply subcommand
setup.js->>FileManifest: Read .codeforge/file-manifest.json
setup.js->>.codeforge/config/: Read source files with variable expansion
setup.js->>setup.js: Resolve destinations per manifest (overwrite policy)
setup.js->>.claude/: Deploy settings, prompts, and rules to container paths
end
deactivate setup.js
User->>User: Configuration applied and ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.devcontainer/scripts/setup-config.sh (1)
31-38:⚠️ Potential issue | 🔴 CriticalFix legacy fallback path construction to match
.devcontainer/config/defaults/structure.The legacy fallback at line 15-19 checks for
.devcontainer/config/defaults/but then setsCONFIG_DIR="${WORKSPACE_ROOT}/.devcontainer/config". The loop at line 31 then tries to access$CONFIG_DIR/config/settings.json, resulting in.devcontainer/config/config/settings.json, which doesn't exist.The actual legacy files are at
.devcontainer/config/defaults/settings.json. Fix by either:
- Setting
CONFIG_DIR="${WORKSPACE_ROOT}/.devcontainer/config/defaults"(line 17), or- Changing the loop to iterate
for file in settings.json keybindings.json main-system-prompt.mdwithout theconfig/prefix (line 31).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-config.sh around lines 31 - 38, The legacy fallback constructs CONFIG_DIR incorrectly — change the assignment that sets CONFIG_DIR from "${WORKSPACE_ROOT}/.devcontainer/config" to "${WORKSPACE_ROOT}/.devcontainer/config/defaults" so the later loop that copies "$CONFIG_DIR/config/settings.json" (and files referenced by the loop) resolves to the actual legacy files, ensuring CONFIG_DIR, WORKSPACE_ROOT and the copy loop (for file in config/settings.json config/keybindings.json config/main-system-prompt.md) remain consistent.
🧹 Nitpick comments (3)
setup.js (2)
112-129: Add error handling for JSON.parse in readChecksums.If the checksum file contains malformed JSON, this will throw an unhandled exception. Consider wrapping in try-catch with a fallback to empty checksums.
🛡️ Proposed fix for JSON parse error handling
const latest = files[files.length - 1]; - return JSON.parse(fs.readFileSync(path.join(checksumsDir, latest), "utf-8")); + try { + return JSON.parse(fs.readFileSync(path.join(checksumsDir, latest), "utf-8")); + } catch (err) { + console.warn("Warning: Could not parse checksum file, treating as fresh install"); + return { files: {} }; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.js` around lines 112 - 129, readChecksums currently calls JSON.parse on the latest checksum file without protection; wrap the fs.readFileSync + JSON.parse for the selected file (use the latest variable and path.join(checksumsDir, latest)) in a try-catch, and on any error return a safe fallback { files: {} } (and optionally log or warn the parsing error) so malformed JSON does not throw an uncaught exception.
516-529: configApply handlesneverandif-changedbut notalwaysoverwrite mode.The file-manifest.json documentation mentions three overwrite modes:
if-changed,always, andnever. Thealwaysmode is not explicitly handled here - files without a matching condition fall through to the copy at line 531, which effectively implementsalwaysbehavior. However, adding explicit handling would improve code clarity.✨ Suggested explicit handling for completeness
if (entry.overwrite === "if-changed" && fs.existsSync(destPath)) { const srcHash = computeChecksum(srcPath); const destHash = computeChecksum(destPath); if (srcHash === destHash) { skipped++; continue; } } + + // overwrite: "always" (default) falls through to copy🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.js` around lines 516 - 529, The overwrite handling currently checks entry.overwrite === "never" and "if-changed" but leaves "always" implicit; add an explicit branch for entry.overwrite === "always" to make intent clear and maintain consistency: when entry.overwrite === "always" and the destination exists, do not increment skipped and proceed to copy (allow overwrite), optionally log "Overwrite: <filename> (always)"; keep the existing computeChecksum-based logic for "if-changed" (using srcPath, destPath, computeChecksum) and preserve the skipped counter only for the "never" and unchanged "if-changed" cases to avoid changing semantics..devcontainer/scripts/setup-migrate-codeforge.sh (1)
38-47: Consider adding error handling for copy and sed operations.The migration copies files and rewrites the manifest without error checking. If these operations fail silently, users may end up with a partially migrated state.
🛡️ Proposed fix to add error handling
# Copy config files from .devcontainer/config/defaults/ → .codeforge/config/ if [ -d "$OLD_DEFAULTS_DIR" ]; then - cp -a "$OLD_DEFAULTS_DIR/." "$CODEFORGE_DIR/config/" - log "Copied config files from defaults/ → .codeforge/config/" + if cp -a "$OLD_DEFAULTS_DIR/." "$CODEFORGE_DIR/config/"; then + log "Copied config files from defaults/ → .codeforge/config/" + else + warn "Failed to copy config files — migration may be incomplete" + fi fi # Copy file-manifest.json, rewriting src paths from defaults/ to config/ if [ -f "$OLD_CONFIG_DIR/file-manifest.json" ]; then - sed 's|"defaults/|"config/|g' "$OLD_CONFIG_DIR/file-manifest.json" > "$CODEFORGE_DIR/file-manifest.json" - log "Copied file-manifest.json (rewrote defaults/ → config/)" + if sed 's|"defaults/|"config/|g' "$OLD_CONFIG_DIR/file-manifest.json" > "$CODEFORGE_DIR/file-manifest.json"; then + log "Copied file-manifest.json (rewrote defaults/ → config/)" + else + warn "Failed to rewrite file-manifest.json — migration may be incomplete" + fi fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/setup-migrate-codeforge.sh around lines 38 - 47, Wrap the copy and sed operations with explicit error handling: after the cp command that uses OLD_DEFAULTS_DIR → CODEFORGE_DIR call out failures by checking the exit status and call the existing log function with a clear error message (including the source and destination variables) and exit non‑zero; do the same for the sed rewrite of OLD_CONFIG_DIR/file-manifest.json → CODEFORGE_DIR/file-manifest.json (check sed/create redirect success, log error with file-manifest.json and relevant env vars, and exit). Ensure you reference the same symbols used in the diff (OLD_DEFAULTS_DIR, CODEFORGE_DIR, OLD_CONFIG_DIR, file-manifest.json, and log) so any failure aborts the script with a helpful log entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/content/docs/reference/troubleshooting.md`:
- Around line 135-141: The "Reset to Defaults" wording is misleading because
`--reset` preserves `.codeforge/` modifications; update the document to either
rename the section (e.g., "Reset Runtime State") and adjust references to
`~/.claude/` and `setup-config.sh`, or keep the title and add explicit steps to
fully restore defaults by deleting or restoring `.codeforge/config/*` (for
example, instruct to remove or replace `.codeforge/config/` with the original
defaults before running `setup-config.sh` or provide a command to copy defaults
from a known source), and clarify how `--reset` behaves vs. a full rebuild (`Dev
Containers: Rebuild Container`) and alias reset steps (`~/.bashrc`, `~/.zshrc`
and `bash /workspaces/.devcontainer/scripts/setup-aliases.sh`).
---
Outside diff comments:
In @.devcontainer/scripts/setup-config.sh:
- Around line 31-38: The legacy fallback constructs CONFIG_DIR incorrectly —
change the assignment that sets CONFIG_DIR from
"${WORKSPACE_ROOT}/.devcontainer/config" to
"${WORKSPACE_ROOT}/.devcontainer/config/defaults" so the later loop that copies
"$CONFIG_DIR/config/settings.json" (and files referenced by the loop) resolves
to the actual legacy files, ensuring CONFIG_DIR, WORKSPACE_ROOT and the copy
loop (for file in config/settings.json config/keybindings.json
config/main-system-prompt.md) remain consistent.
---
Nitpick comments:
In @.devcontainer/scripts/setup-migrate-codeforge.sh:
- Around line 38-47: Wrap the copy and sed operations with explicit error
handling: after the cp command that uses OLD_DEFAULTS_DIR → CODEFORGE_DIR call
out failures by checking the exit status and call the existing log function with
a clear error message (including the source and destination variables) and exit
non‑zero; do the same for the sed rewrite of OLD_CONFIG_DIR/file-manifest.json →
CODEFORGE_DIR/file-manifest.json (check sed/create redirect success, log error
with file-manifest.json and relevant env vars, and exit). Ensure you reference
the same symbols used in the diff (OLD_DEFAULTS_DIR, CODEFORGE_DIR,
OLD_CONFIG_DIR, file-manifest.json, and log) so any failure aborts the script
with a helpful log entry.
In `@setup.js`:
- Around line 112-129: readChecksums currently calls JSON.parse on the latest
checksum file without protection; wrap the fs.readFileSync + JSON.parse for the
selected file (use the latest variable and path.join(checksumsDir, latest)) in a
try-catch, and on any error return a safe fallback { files: {} } (and optionally
log or warn the parsing error) so malformed JSON does not throw an uncaught
exception.
- Around line 516-529: The overwrite handling currently checks entry.overwrite
=== "never" and "if-changed" but leaves "always" implicit; add an explicit
branch for entry.overwrite === "always" to make intent clear and maintain
consistency: when entry.overwrite === "always" and the destination exists, do
not increment skipped and proceed to copy (allow overwrite), optionally log
"Overwrite: <filename> (always)"; keep the existing computeChecksum-based logic
for "if-changed" (using srcPath, destPath, computeChecksum) and preserve the
skipped counter only for the "never" and unchanged "if-changed" cases to avoid
changing semantics.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
.codeforge/.codeforge-preserve.codeforge/config/ccstatusline-settings.json.codeforge/config/keybindings.json.codeforge/config/main-system-prompt.md.codeforge/config/orchestrator-system-prompt.md.codeforge/config/rules/session-search.md.codeforge/config/rules/spec-workflow.md.codeforge/config/rules/workspace-scope.md.codeforge/config/settings.json.codeforge/config/writing-system-prompt.md.codeforge/file-manifest.json.codeforge/scripts/connect-external-terminal.ps1.codeforge/scripts/connect-external-terminal.sh.devcontainer/.env.example.devcontainer/CHANGELOG.md.devcontainer/CLAUDE.md.devcontainer/README.md.devcontainer/docs/configuration-reference.md.devcontainer/docs/keybindings.md.devcontainer/docs/plugins.md.devcontainer/features/ccstatusline/install.sh.devcontainer/features/tmux/README.md.devcontainer/plugins/devs-marketplace/plugins/agent-system/agents/claude-guide.md.devcontainer/scripts/setup-aliases.sh.devcontainer/scripts/setup-config.sh.devcontainer/scripts/setup-migrate-claude.sh.devcontainer/scripts/setup-migrate-codeforge.sh.devcontainer/scripts/setup.sh.gitignoreCLAUDE.mddocs/astro.config.mjsdocs/src/content/docs/customization/configuration.mddocs/src/content/docs/customization/index.mddocs/src/content/docs/customization/keybindings.mddocs/src/content/docs/customization/optional-features.mddocs/src/content/docs/customization/rules.mddocs/src/content/docs/customization/system-prompts.mddocs/src/content/docs/getting-started/installation.mddocs/src/content/docs/reference/architecture.mddocs/src/content/docs/reference/changelog.mddocs/src/content/docs/reference/environment.mddocs/src/content/docs/reference/index.mddocs/src/content/docs/reference/troubleshooting.mdpackage.jsonsetup.jstest.js
💤 Files with no reviewable changes (3)
- .devcontainer/docs/keybindings.md
- .devcontainer/docs/plugins.md
- .devcontainer/docs/configuration-reference.md
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
docs/src/content/docs/reference/architecture.md (2)
163-165: Consider explaining the different system prompt modes.The documentation lists three system prompts (main, orchestrator, writing) but doesn't explain when each is used or what distinguishes them. Since orchestrator-system-prompt.md is newly introduced in this PR, users would benefit from a brief explanation of these modes.
Consider adding a comment or short note explaining the purpose of each prompt type, similar to the explanatory comments for other components in the structure.
📝 Example addition to clarify prompt modes
.codeforge/ +-- file-manifest.json # Declarative config deployment rules +-- config/ | +-- settings.json # Claude Code settings (model, plugins, env vars) | +-- keybindings.json # Keyboard shortcuts -| +-- main-system-prompt.md # Development system prompt -| +-- orchestrator-system-prompt.md # Orchestrator mode system prompt -| +-- writing-system-prompt.md # Writing mode system prompt +| +-- main-system-prompt.md # Default development system prompt +| +-- orchestrator-system-prompt.md # Multi-agent coordination mode +| +-- writing-system-prompt.md # Documentation and writing tasks mode | +-- rules/ # Default rules deployed to .claude/rules/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/content/docs/reference/architecture.md` around lines 163 - 165, Add a short explanatory note after the listed prompts clarifying when each is used: describe main-system-prompt.md as the default/dev system prompt for general behavior and local development, orchestrator-system-prompt.md as the prompt driving orchestration/decision-making mode (used when the orchestrator coordinates tools or agents), and writing-system-prompt.md as the prompt tuned for content-generation/writing mode; reference the three filenames (main-system-prompt.md, orchestrator-system-prompt.md, writing-system-prompt.md) so readers can open them for details.
210-219: Consider documenting the checksum-based preservation mechanism.The PR introduces SHA-256 checksum-based modification detection to preserve user-modified files during updates, which is a significant architectural decision affecting the "Data Flow" during container startup. Consider adding a brief mention of this mechanism either in the "Design Principles" section (as a new principle about respecting user modifications) or in this startup flow section.
This would help users understand why updates don't overwrite their customizations and how the system handles conflicts (writing .default files when both default and user modifications have changed).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/content/docs/reference/architecture.md` around lines 210 - 219, Add a short mention in the architecture docs about the new SHA-256 checksum-based preservation mechanism: describe that during container startup the setup-*.sh scripts (e.g., setup-config.sh, setup-plugins.sh, setup-aliases.sh, setup-projects.sh) compute SHA-256 checksums of default files and compare them to installed copies to avoid overwriting user-modified files; note the conflict policy that if both default and user files change the updater writes a .default backup and preserves the user file; place this either as a new bullet in "Design Principles" (respect user modifications) or as an extra step in the startup "Data Flow" section so readers understand why updates do not overwrite customizations..gitignore (1)
66-66: Consider making the comment more complete.The comment mentions "except devcontainer and codeforge" but the unignore rules below also include
.git/,.github/,.gitignore,.gitattributes, and.npmignore. While the comment captures the main intent, it could be more accurate.📝 Suggested comment improvements
-# All hidden directories except devcontainer and codeforge +# All hidden files and directories (with specific exceptions below)or
-# All hidden directories except devcontainer and codeforge +# Ignore all hidden files/dirs, then selectively unignore devcontainer, codeforge, git, etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 66, Update the .gitignore comment that currently reads "All hidden directories except devcontainer and codeforge" to accurately list all exceptions used by the rules (include .git/, .github/, .gitignore, .gitattributes, .npmignore, devcontainer, codeforge) so the comment matches the unignore patterns; locate the comment near the hidden-directory rules and replace it with a concise, accurate line referencing those specific exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/README.md:
- Line 435: The README links to GitHub Pages (the five URLs listed) are pointing
to anexileddev.github.io/CodeForge but that site is not live via HTTPS; verify
the docs site is deployed to GitHub Pages and either update the URLs in
.devcontainer/README.md to the correct public domain (custom domain if used) or
change them to relative/local links that reference the local docs content (e.g.,
paths under docs/src/content/docs/) so they work pre-deployment; update the five
affected links (reference/troubleshooting, customization/configuration, plugins,
customization/optional-features, customization/keybindings) accordingly and
confirm each resolves over HTTPS.
In @.devcontainer/scripts/setup.sh:
- Around line 49-50: The script unconditionally overwrites CODEFORGE_DIR; change
the assignment at the top so it only sets CODEFORGE_DIR when not already defined
(use shell default-assignment/parameter expansion instead of unconditional
assignment). Update the assignment that currently references WORKSPACE_ROOT and
CODEFORGE_DIR so existing user-provided CODEFORGE_DIR from the env is preserved,
and keep the unset CONFIG_SOURCE_DIR line as-is; target the statement that sets
CODEFORGE_DIR and use the same default-assignment pattern used elsewhere in the
script (e.g., the pattern used around the other default on line 59).
In `@docs/src/content/docs/reference/architecture.md`:
- Around line 158-174: Update the documented .codeforge/ tree to include the two
missing directories present in the implementation: add a `.markers/` entry (used
to track migration state like `v2-migrated`) and a `.checksums/` entry (used to
store file hashes for change detection) alongside the existing entries such as
file-manifest.json, config/, scripts/, and .codeforge-preserve so the docs match
setup.js and .gitignore.
In `@setup.js`:
- Around line 515-525: The code builds srcPath and destPath from untrusted
manifest fields (entry.src, entry.dest) inside configApply(); fix by resolving
and validating both paths before using them: compute const resolvedSrc =
path.resolve(codeforgeDir, entry.src) and ensure
resolvedSrc.startsWith(path.resolve(codeforgeDir) + path.sep) (skip/log and
continue if not), and for the destination compute const resolvedDestDir =
path.resolve(allowedDestBase || some configured install base,
expandVars(entry.dest)) and ensure
resolvedDestDir.startsWith(path.resolve(allowedDestBase) + path.sep) (skip/log
and continue if not) before creating directories or writing files; keep using
path.basename(entry.src) for filename and join with validated resolvedDestDir to
form destPath.
- Around line 118-127: readChecksums currently sorts filenames lexicographically
causing semantic versions like "2.10.0.json" to sort before "2.9.0.json"; update
the sorting to be semver-aware by parsing the version numbers from each filename
(strip the ".json" suffix) and comparing numeric version parts (major, minor,
patch) so that files is ordered by true version and latest = files[files.length
- 1] picks the newest snapshot; modify the sort call that populates files (and
reference variables files, latest, checksumsDir within readChecksums) to use
this numeric/semver comparison (or use a semver compare helper) before selecting
latest.
---
Nitpick comments:
In @.gitignore:
- Line 66: Update the .gitignore comment that currently reads "All hidden
directories except devcontainer and codeforge" to accurately list all exceptions
used by the rules (include .git/, .github/, .gitignore, .gitattributes,
.npmignore, devcontainer, codeforge) so the comment matches the unignore
patterns; locate the comment near the hidden-directory rules and replace it with
a concise, accurate line referencing those specific exceptions.
In `@docs/src/content/docs/reference/architecture.md`:
- Around line 163-165: Add a short explanatory note after the listed prompts
clarifying when each is used: describe main-system-prompt.md as the default/dev
system prompt for general behavior and local development,
orchestrator-system-prompt.md as the prompt driving
orchestration/decision-making mode (used when the orchestrator coordinates tools
or agents), and writing-system-prompt.md as the prompt tuned for
content-generation/writing mode; reference the three filenames
(main-system-prompt.md, orchestrator-system-prompt.md, writing-system-prompt.md)
so readers can open them for details.
- Around line 210-219: Add a short mention in the architecture docs about the
new SHA-256 checksum-based preservation mechanism: describe that during
container startup the setup-*.sh scripts (e.g., setup-config.sh,
setup-plugins.sh, setup-aliases.sh, setup-projects.sh) compute SHA-256 checksums
of default files and compare them to installed copies to avoid overwriting
user-modified files; note the conflict policy that if both default and user
files change the updater writes a .default backup and preserves the user file;
place this either as a new bullet in "Design Principles" (respect user
modifications) or as an extra step in the startup "Data Flow" section so readers
understand why updates do not overwrite customizations.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.devcontainer/README.md.devcontainer/scripts/setup.sh.gitignoredocs/src/content/docs/reference/architecture.mdsetup.js
There was a problem hiding this comment.
♻️ Duplicate comments (1)
setup.js (1)
535-545:⚠️ Potential issue | 🟠 MajorValidate manifest-resolved paths before copy to prevent path traversal.
The path traversal vulnerability remains unaddressed.
entry.srccan escape.codeforge/using../sequences, andentry.destafter variable expansion isn't validated against allowed boundaries. An attacker controllingfile-manifest.jsoncan read or write files outside intended directories.Proposed fix
+ const codeforgeRoot = path.resolve(codeforgeDir); + const claudeRoot = path.resolve(claudeConfigDir); + for (const entry of entries) { if (entry.enabled === false) { skipped++; continue; } if (entry.overwrite && !validOverwrite.includes(entry.overwrite)) { console.log( ' Warning: Unknown overwrite value "' + entry.overwrite + '" for ' + entry.src + ', defaulting to "always"', ); } - const srcPath = path.join(codeforgeDir, entry.src); + const srcPath = path.resolve(codeforgeDir, entry.src); + if (!srcPath.startsWith(codeforgeRoot + path.sep) && srcPath !== codeforgeRoot) { + console.log(" Skip: " + entry.src + " (invalid source path)"); + skipped++; + continue; + } if (!fs.existsSync(srcPath)) { console.log(" Skip: " + entry.src + " (not found)"); skipped++; continue; } const destDir = expandVars(entry.dest); + const resolvedDestDir = path.resolve(destDir); + if (!resolvedDestDir.startsWith(claudeRoot + path.sep) && resolvedDestDir !== claudeRoot) { + console.log(" Skip: " + entry.dest + " (outside allowed destination)"); + skipped++; + continue; + } const filename = entry.destFilename || path.basename(entry.src); - const destPath = path.join(destDir, filename); + const destPath = path.join(resolvedDestDir, filename); fs.mkdirSync(destDir, { recursive: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.js` around lines 535 - 545, Validate resolved absolute paths to prevent path traversal: resolve srcPath and destPath with path.resolve and ensure srcPath starts with the intended codeforgeDir root (use path.resolve(codeforgeDir) as the canonical base) and reject entries where path.resolve(codeforgeDir, entry.src) escapes that base; likewise resolve destDir after expandVars and ensure path.resolve(destDir, entry.destFilename || path.basename(entry.src)) is within the allowed destination base (project or sandbox root) before calling fs.mkdirSync or performing any copy; on validation failure, log and skip the entry instead of operating on the path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@setup.js`:
- Around line 535-545: Validate resolved absolute paths to prevent path
traversal: resolve srcPath and destPath with path.resolve and ensure srcPath
starts with the intended codeforgeDir root (use path.resolve(codeforgeDir) as
the canonical base) and reject entries where path.resolve(codeforgeDir,
entry.src) escapes that base; likewise resolve destDir after expandVars and
ensure path.resolve(destDir, entry.destFilename || path.basename(entry.src)) is
within the allowed destination base (project or sandbox root) before calling
fs.mkdirSync or performing any copy; on validation failure, log and skip the
entry instead of operating on the path.
Centralizes all user-facing config files into .codeforge/, separating user customization from .devcontainer/ framework internals. Adds checksum-based modification detection so updates preserve user changes. - Move config files from .devcontainer/config/defaults/ to .codeforge/config/ - Move file-manifest.json to .codeforge/ - Move terminal scripts to .codeforge/scripts/ - Add checksum-based sync (SHA-256) for --force and --reset - Add auto-migration script for existing installations - Add codeforge config apply CLI subcommand - Add codeforge alias to setup-aliases.sh - Update setup.sh with CODEFORGE_DIR env var and deprecation guard - Update setup-config.sh to read from .codeforge/ - Bump version to 2.0.0 - Update all documentation and docs site references
Review fixes from 4-agent code review: - setup.js: Handle corrupted checksums JSON gracefully (try/catch) - setup.js: Generate checksums from source, not destination on fresh install - setup.js: Use regex with /g flag in expandVars for multiple occurrences - setup.sh: Fix variable ordering — set CODEFORGE_DIR before CONFIG_SOURCE_DIR - setup.sh: Unset CONFIG_SOURCE_DIR after deprecation guard fires - .gitignore: Remove duplicate .devcontainer/ un-ignore and stale config paths - architecture.md: Add missing orchestrator-system-prompt.md and migration step - README.md: Fix dead links to removed docs/ directory, point to docs site
- readChecksums: Sort version filenames by numeric semver components instead of lexicographic order (1.10.0 now correctly sorts after 1.9.0) - configApply: Validate overwrite field against known values (always, if-changed, never) and warn on unrecognized values
The checksum system in syncCodeforgeDirectory automatically preserves user-modified files. This file was never read by any code — removing to avoid implying functionality that doesn't exist.
- Add path traversal validation in configApply() for source and destination paths in file-manifest.json - Use default-assignment for CODEFORGE_DIR in deprecation guard - Replace old GitHub Pages URLs with custom domain (codeforge.core-directive.com) - Add .checksums/ and .markers/ to architecture docs tree - Clarify "How to Reset" section in troubleshooting docs - Fix "merged" → "migrated content from" in changelog wording
73593e9 to
17f38c9
Compare
The merge of PR #33 included a premature version bump to 2.0.0, which triggered the automated Release workflow. The npm package has been unpublished and the GitHub release/tag deleted.
Summary
.codeforge/, separating user-facing customization from.devcontainer/framework internals--forceand--resetupdates preserve user-modified files while delivering new defaults.devcontainer/config/defaults/without.codeforge/and creates it automaticallycodeforge config applydeploys.codeforge/config/files to~/.claude/(same as container start)Changes
New directory structure
Shell scripts
setup.sh— newCODEFORGE_DIRenv var, deprecation guard forCONFIG_SOURCE_DIR, auto-migration triggersetup-config.sh— reads from.codeforge/with legacy fallbacksetup-migrate-codeforge.sh— new one-time migration scriptsetup-migrate-claude.sh— updated marker location with backward compatsetup-aliases.sh— addedcodeforgealiasNode.js installer (
setup.js).devcontainer/and.codeforge/, generates checksums--forceuses checksum-based sync: unmodified files overwritten, user-modified files preserved with.defaultsuffix--resetwipes.devcontainer/but preserves user-modified.codeforge/filesconfig applysubcommandDocumentation (16 files)
All
config/defaults/references updated to.codeforge/config/across docs site, READMEs, CLAUDE.md, and changelogs.Test plan
node test.jspasses (18/18 tests)npx codeforgecreates both directories with checksums.defaultwritten.devcontainer/wiped,.codeforge/user mods preserved.codeforge/config/to~/.claude/.codeforge/gets it auto-createdcodeforge config applydeploys files inside containerconfig/defaults/references in non-changelog docsSummary by CodeRabbit
Release Notes
New Features
.codeforge/user customization directory with automatic migration from legacy pathscodeforge config applycommand for configuration deploymentDocumentation
Chores