Conversation
- Run marked output through sanitize-html so AI-generated/untrusted post Markdown can't inject <script>, event handlers, or javascript: URLs (verified: script/onerror/javascript: stripped, normal content kept). - Replace the inline one-liner serve script with scripts/serve.mjs (readable, adds path-traversal guard and proper MIME types); drop the dead s.writeCode?0:0 expression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- scripts/generate.mjs: read recent memory/ notes, distill into one post, write to posts/ with front matter (slug from title, date fallback). - scripts/providers/: pluggable backends selected by env — anthropic (Claude, claude-opus-4-8, adaptive thinking), openai, and a zero-dependency dryrun stub so the pipeline runs with no API key. SDKs are optionalDependencies, imported dynamically. - memory/ with an example note as the input source. - .github/workflows/generate.yml: scheduled/manual generate -> PR to develop for human review (never publishes directly). - npm run generate; README + AGENTS document the memory->AI->PR flow. - Verified end to end: dryrun generate -> build renders the post; missing ANTHROPIC_API_KEY errors clearly instead of failing silently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
📝 WalkthroughWalkthroughAdds a complete AI-powered blog post generation pipeline to OpenBlog. A new CLI ( ChangesAI Post Generation Pipeline
Sequence Diagram(s)sequenceDiagram
participant Cron as Schedule / Manual Dispatch
participant GHA as GitHub Actions
participant Gen as scripts/generate.mjs
participant Memory as memory/*.md
participant Provider as AI Provider (Anthropic/OpenAI/dryrun)
participant Posts as posts/*.md
participant PR as Pull Request (develop)
Cron->>GHA: trigger generate.yml
GHA->>GHA: npm ci + conditional SDK install
GHA->>Gen: node scripts/generate.mjs --date YYYY-MM-DD
Gen->>Memory: scan + parse front matter, filter by cutoff
Memory-->>Gen: sorted notes[]
Gen->>Provider: generate({notes, date})
Provider-->>Gen: {title, tags, summary, body}
Gen->>Posts: write YYYY-MM-DD-slug.md
GHA->>PR: create-pull-request → develop (label: ai-generated)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
148-148:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the branch guidance with
develop.This sentence says PRs should target
main, but the repo policy elsewhere usesdevelopas the default work branch. Please update the README so contributors don't follow the wrong target. As per coding guidelines, "Default to the 'develop' branch for all work and pushes. Do NOT push or open PRs againstmainunless the task explicitly says to release/publish."🤖 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 `@README.md` at line 148, The README line currently instructs contributors to submit Pull Requests to `main`, but the repository policy requires PRs to target `develop` as the default work branch. Update the sentence at line 148 to change the PR target guidance from `main` to `develop` to align with the coding guidelines that state all work and PRs should default to `develop` unless the task explicitly requires a release or publish to `main`.Source: Learnings
🧹 Nitpick comments (2)
scripts/providers/anthropic.mjs (1)
41-42: Thethinking: { type: "adaptive" }parameter andmax_tokens: 8000are both supported in@anthropic-ai/sdkversion 0.40.0 for claude-opus-4-8. However, the documentation recommends setting a large max output token budget when using adaptive thinking, and 8000 is significantly below the 128,000 maximum for this model. Consider increasingmax_tokensto allow the model sufficient capacity for thinking and response generation when using adaptive thinking mode.🤖 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 `@scripts/providers/anthropic.mjs` around lines 41 - 42, The max_tokens value of 8000 is significantly below the recommended threshold for adaptive thinking mode in the claude-opus-4-8 model, which supports up to 128,000 tokens. Increase the max_tokens parameter in the configuration object (currently set to 8000) to a substantially larger value that provides adequate capacity for both the model's thinking process and response generation when using the adaptive thinking mode.memory/2026-06-12-example.md (1)
8-11: ⚡ Quick winSplit this into one-thought-per-file notes in
memory/.This entry contains multiple separate thoughts in one file; split them into separate
memory/*.mdnotes so each thought is independently ingestible.As per coding guidelines:
memory/*.md: “one note per thought per file” and includedate/tagsfront matter.🤖 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 `@memory/2026-06-12-example.md` around lines 8 - 11, The memory/2026-06-12-example.md file contains four separate and distinct thoughts combined into a single note, which violates the one-thought-per-file guideline. Split each of the four bullet-point thoughts into its own individual .md file in the memory/ directory (creating 4 new files total). For each new file, include the appropriate front matter with date and tags metadata as specified in the memory directory guidelines, and ensure each file contains only a single cohesive thought with its content.Source: Coding guidelines
🤖 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 @.github/workflows/generate.yml:
- Around line 33-37: Replace the mutable version tags with full-length commit
SHAs for the three third-party GitHub Actions: actions/checkout@v4,
actions/setup-node@v4, and peter-evans/create-pull-request@v6. For each action,
look up the specific commit SHA for the version being used and replace the tag
(e.g., `@v4` or `@v6`) with the complete commit hash in the format
@<full-commit-sha>. This pins the actions to immutable commit references,
improving supply-chain security and ensuring workflow reproducibility.
- Around line 66-67: The shell substitution syntax $(date -u +%F) in the
commit-message and title inputs to the peter-evans/create-pull-request@v6 action
will not be evaluated because these inputs are not executed in a shell context
and will render literally. Replace the shell substitution with a GitHub Actions
expression instead. You can either use a built-in expression like ${{
github.run_id }} if that meets your needs, or create a preceding step that
generates the date using run with shell syntax, assigns it to an output variable
with id and outputs syntax, and then reference that output in the commit-message
and title inputs using ${{ steps.step_id.outputs.variable_name }} format.
- Around line 33-35: The actions/checkout@v4 action in the workflow is
configured without disabling credential persistence, which leaves the GitHub
token in Git configuration for subsequent steps and can conflict with the
downstream peter-evans/create-pull-request@v6 action that manages its own
authentication. Add the persist-credentials: false parameter to the with section
of the actions/checkout@v4 action to disable this default behavior and reduce
the credential blast radius.
In `@scripts/generate.mjs`:
- Around line 26-37: The parseArgs function accepts command-line arguments for
--days, --max, and --date without any validation, allowing invalid values
(negative numbers, NaN, malformed dates) to silently pass through to filtering
logic. Add validation checks in the parseArgs function after each argument
assignment to ensure: days is a positive integer, max is a positive integer, and
date is a valid date string (if provided). Additionally, verify that the
validated arguments are properly used at lines 54-58 and 98-107 where they are
consumed for filtering and output operations, ensuring these locations respect
the validation constraints or add additional defensive checks if the arguments
are used in different contexts.
- Around line 87-95: The uniquePath function can return an existing file path
when collisions for date-slug through date-slug-99 all exist, causing unintended
overwrites. After the for loop (which iterates from n=2 to n<100) completes
without finding a unique path, the function currently returns the last candidate
value regardless of whether it exists. Fix this by checking if the final
candidate path exists after the loop completes—if it does, either throw an error
indicating too many collisions, or extend the search range beyond 100 to
continue looking for an available filename, ensuring the function never returns
a path that already exists unless the force flag is explicitly set to true.
In `@scripts/serve.mjs`:
- Around line 27-35: The path validation has two security/stability issues: the
`filePath.startsWith(PUBLIC_DIR)` check is vulnerable to prefix-confusion
attacks (e.g., `/publicity` bypasses the guard) and `decodeURIComponent` throws
unhandled exceptions on malformed percent-encoding. To fix this, wrap the
`decodeURIComponent` call in a try-catch block to respond with a 400 Bad Request
on invalid input, and harden the path check by using `resolve()` to normalize
both `filePath` and `PUBLIC_DIR` before comparing, ensuring the resolved
filePath actually starts with the normalized PUBLIC_DIR directory path rather
than just matching the string prefix.
---
Outside diff comments:
In `@README.md`:
- Line 148: The README line currently instructs contributors to submit Pull
Requests to `main`, but the repository policy requires PRs to target `develop`
as the default work branch. Update the sentence at line 148 to change the PR
target guidance from `main` to `develop` to align with the coding guidelines
that state all work and PRs should default to `develop` unless the task
explicitly requires a release or publish to `main`.
---
Nitpick comments:
In `@memory/2026-06-12-example.md`:
- Around line 8-11: The memory/2026-06-12-example.md file contains four separate
and distinct thoughts combined into a single note, which violates the
one-thought-per-file guideline. Split each of the four bullet-point thoughts
into its own individual .md file in the memory/ directory (creating 4 new files
total). For each new file, include the appropriate front matter with date and
tags metadata as specified in the memory directory guidelines, and ensure each
file contains only a single cohesive thought with its content.
In `@scripts/providers/anthropic.mjs`:
- Around line 41-42: The max_tokens value of 8000 is significantly below the
recommended threshold for adaptive thinking mode in the claude-opus-4-8 model,
which supports up to 128,000 tokens. Increase the max_tokens parameter in the
configuration object (currently set to 8000) to a substantially larger value
that provides adequate capacity for both the model's thinking process and
response generation when using the adaptive thinking mode.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 96d89c16-d978-4698-bdc2-cc761eec4a06
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/workflows/generate.ymlAGENTS.mdREADME.mdmemory/.gitkeepmemory/2026-06-12-example.mdpackage.jsonscripts/build.mjsscripts/generate.mjsscripts/providers/anthropic.mjsscripts/providers/dryrun.mjsscripts/providers/index.mjsscripts/providers/openai.mjsscripts/providers/prompt.mjsscripts/serve.mjs
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: develop |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the workflow file
find . -name "generate.yml" -o -name "generate.yaml" | head -20Repository: xiami303/OpenBlog
Length of output: 94
🏁 Script executed:
# Also check the repository structure for workflow files
git ls-files .github/workflows/ 2>/dev/null | head -20Repository: xiami303/OpenBlog
Length of output: 122
🏁 Script executed:
# Read the full generate.yml workflow file
cat -n .github/workflows/generate.ymlRepository: xiami303/OpenBlog
Length of output: 2999
🌐 Web query:
actions/checkout persist-credentials default behavior GitHub Actions
💡 Result:
The default behavior of the persist-credentials input in the actions/checkout GitHub Action is true [1][2][3]. When set to true (the default), the action automatically configures the local Git repository with the provided GitHub token or SSH key [1][4][3]. This allows your workflow scripts to execute authenticated Git commands (such as git fetch or git push) without requiring manual credential configuration [1][5][6]. For security, the action now stores these credentials in a separate file under $RUNNER_TEMP rather than directly within the.git/config file, and the token is automatically removed during post-job cleanup [1][6]. If you do not want these credentials persisted in the Git configuration, you can opt out by setting persist-credentials to false in your workflow file [1][5][7].
Citations:
- 1: https://github.com/actions/checkout/?tab=readme-ov-file
- 2: https://github.com/actions/checkout/blob/34e114876b0b11c390a56381ad16ebd13914f8d5/action.yml
- 3: https://github.com/actions/checkout/blob/main/action.yml
- 4: https://github.com/actions/checkout/blob/85e6279cec87321a52edac9c87bce653a07cf6c2/README.md
- 5: https://github.com/actions/checkout
- 6: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 7: https://github.com/marketplace/actions/checkout
🌐 Web query:
peter-evans/create-pull-request credentials persist-credentials security
💡 Result:
When using the peter-evans/create-pull-request action in workflows that also use actions/checkout, you may encounter an HTTP 400 error described as a "Duplicate header: Authorization" [1][2][3]. This occurs because actions/checkout (specifically from version 6 onwards) persists a git credential by default, while peter-evans/create-pull-request also injects its own authentication header [4][2][3]. When Git attempts to push changes, both headers are sent, leading GitHub to reject the request [1][2][3]. To resolve this, the recommended approach is to set persist-credentials to false in your actions/checkout step [1][2][3]. This prevents the checkout action from storing credentials, leaving the peter-evans/create-pull-request action to handle the necessary authentication for the subsequent push [1][2]. Example workflow configuration: - uses: actions/checkout@v6 with: persist-credentials: false - name: Create Pull Request uses: peter-evans/create-pull-request@v8 with: token: ${{ secrets.GITHUB_TOKEN }} While the peter-evans/create-pull-request action has implemented internal measures to attempt to mitigate compatibility issues with actions/checkout@v6 (such as temporarily hiding credential files) [4][5][6], setting persist-credentials: false remains a common and effective practice to ensure reliable authentication and avoid duplicate header conflicts [1][2][3].
Citations:
- 1: Nox-HQ/nox@c7d7d95
- 2: fix(build-sdk): resolve duplicate Authorization header in create-pull-request step tserra30/Github-Copilot-SDK-integration#141
- 3: ci(bump-versions): set persist-credentials=false to fix actions/checkout@v6 + create-pull-request@v6 duplicate-Authorization 400 ligate-io/ligate-chain#501
- 4: Fix compatibility with actions/checkout@v6 peter-evans/create-pull-request#4230
- 5: peter-evans/create-pull-request@84ae59a
- 6: Fix compatibility with actions/checkout@v6 peter-evans/create-pull-request#4230
Disable credential persistence on checkout.
actions/checkout@v4 keeps credentials enabled by default, persisting the GitHub token in Git configuration for subsequent steps. Since this workflow uses peter-evans/create-pull-request@v6 which manages its own authentication, set persist-credentials: false to narrow the credential blast radius and avoid authentication conflicts.
Suggested fix
- uses: actions/checkout@v4
with:
ref: develop
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 33-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 33-33: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/generate.yml around lines 33 - 35, The actions/checkout@v4
action in the workflow is configured without disabling credential persistence,
which leaves the GitHub token in Git configuration for subsequent steps and can
conflict with the downstream peter-evans/create-pull-request@v6 action that
manages its own authentication. Add the persist-credentials: false parameter to
the with section of the actions/checkout@v4 action to disable this default
behavior and reduce the credential blast radius.
Sources: MCP tools, Linters/SAST tools
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: develop | ||
|
|
||
| - uses: actions/setup-node@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/generate.ymlRepository: xiami303/OpenBlog
Length of output: 2999
Pin the third-party actions to commit SHAs.
actions/checkout@v4 (line 33), actions/setup-node@v4 (line 37), and peter-evans/create-pull-request@v6 (line 61) use mutable tags. GitHub recommends pinning Actions to full-length commit SHAs to reduce supply-chain risk and ensure workflow reproducibility. (docs.github.com)
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 33-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 33-33: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 37-37: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/generate.yml around lines 33 - 37, Replace the mutable
version tags with full-length commit SHAs for the three third-party GitHub
Actions: actions/checkout@v4, actions/setup-node@v4, and
peter-evans/create-pull-request@v6. For each action, look up the specific commit
SHA for the version being used and replace the tag (e.g., `@v4` or `@v6`) with the
complete commit hash in the format @<full-commit-sha>. This pins the actions to
immutable commit references, improving supply-chain security and ensuring
workflow reproducibility.
Sources: MCP tools, Linters/SAST tools
| commit-message: "AI: new post from memory ($(date -u +%F))" | ||
| title: "AI draft: new post from memory ($(date -u +%F))" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/generate.yml | head -80 | tail -20Repository: xiami303/OpenBlog
Length of output: 1059
🌐 Web query:
peter-evans/create-pull-request action shell substitution commit-message title parameter evaluation
💡 Result:
The peter-evans/create-pull-request GitHub action does not perform internal shell substitution on its inputs [1][2]. Instead, it relies on the standard GitHub Actions workflow syntax for processing values before they are passed to the action [1]. To use variables, environment values, or command outputs within the title or commit-message parameters, you must use the
Citations:
- 1: https://github.com/peter-evans/create-pull-request
- 2: https://github.com/peter-evans/create-pull-request/blob/main/README.md
- 3: https://github.com/peter-evans/create-pull-request/tree/v6.0.0
Replace shell substitution with GitHub Actions expression or output variable.
$(date -u +%F) is shell syntax that only evaluates in run steps. The commit-message and title inputs to peter-evans/create-pull-request@v6 are not executed in a shell, so this will render literally. Use a GitHub Actions expression like ${{ github.run_id }} or generate the date in a preceding step and reference it via ${{ steps.step_id.outputs.variable_name }}.
🤖 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 @.github/workflows/generate.yml around lines 66 - 67, The shell substitution
syntax $(date -u +%F) in the commit-message and title inputs to the
peter-evans/create-pull-request@v6 action will not be evaluated because these
inputs are not executed in a shell context and will render literally. Replace
the shell substitution with a GitHub Actions expression instead. You can either
use a built-in expression like ${{ github.run_id }} if that meets your needs, or
create a preceding step that generates the date using run with shell syntax,
assigns it to an output variable with id and outputs syntax, and then reference
that output in the commit-message and title inputs using ${{
steps.step_id.outputs.variable_name }} format.
Source: MCP tools
| function parseArgs(argv) { | ||
| const args = { days: 7, max: 20, force: false }; | ||
| for (let i = 0; i < argv.length; i++) { | ||
| const a = argv[i]; | ||
| if (a === "--date") args.date = argv[++i]; | ||
| else if (a === "--provider") args.provider = argv[++i]; | ||
| else if (a === "--days") args.days = Number(argv[++i]); | ||
| else if (a === "--max") args.max = Number(argv[++i]); | ||
| else if (a === "--force") args.force = true; | ||
| else if (a === "--help" || a === "-h") args.help = true; | ||
| } | ||
| return args; |
There was a problem hiding this comment.
Validate CLI inputs before they reach note filtering.
--days, --max, and --date are used without validation; invalid values (e.g., --max NaN, negative days, bad date) can silently produce incorrect filtering/output behavior.
Suggested fix
function parseArgs(argv) {
const args = { days: 7, max: 20, force: false };
+ const takeValue = (flag, iRef) => {
+ const v = argv[iRef + 1];
+ if (!v || v.startsWith("--")) throw new Error(`Missing value for ${flag}`);
+ return v;
+ };
+
for (let i = 0; i < argv.length; i++) {
const a = argv[i];
- if (a === "--date") args.date = argv[++i];
- else if (a === "--provider") args.provider = argv[++i];
- else if (a === "--days") args.days = Number(argv[++i]);
- else if (a === "--max") args.max = Number(argv[++i]);
+ if (a === "--date") args.date = takeValue("--date", i), i++;
+ else if (a === "--provider") args.provider = takeValue("--provider", i), i++;
+ else if (a === "--days") args.days = Number(takeValue("--days", i)), i++;
+ else if (a === "--max") args.max = Number(takeValue("--max", i)), i++;
else if (a === "--force") args.force = true;
else if (a === "--help" || a === "-h") args.help = true;
}
+
+ if (!Number.isInteger(args.days) || args.days < 0) {
+ throw new Error("--days must be an integer >= 0");
+ }
+ if (!Number.isInteger(args.max) || args.max <= 0) {
+ throw new Error("--max must be an integer > 0");
+ }
+ if (args.date && Number.isNaN(new Date(args.date).getTime())) {
+ throw new Error("--date must be YYYY-MM-DD");
+ }
+
return args;
}Also applies to: 54-58, 98-107
🤖 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 `@scripts/generate.mjs` around lines 26 - 37, The parseArgs function accepts
command-line arguments for --days, --max, and --date without any validation,
allowing invalid values (negative numbers, NaN, malformed dates) to silently
pass through to filtering logic. Add validation checks in the parseArgs function
after each argument assignment to ensure: days is a positive integer, max is a
positive integer, and date is a valid date string (if provided). Additionally,
verify that the validated arguments are properly used at lines 54-58 and 98-107
where they are consumed for filtering and output operations, ensuring these
locations respect the validation constraints or add additional defensive checks
if the arguments are used in different contexts.
| async function uniquePath(date, slug, force) { | ||
| let candidate = join(POSTS_DIR, `${date}-${slug}.md`); | ||
| if (force || !(await exists(candidate))) return candidate; | ||
| for (let n = 2; n < 100; n++) { | ||
| candidate = join(POSTS_DIR, `${date}-${slug}-${n}.md`); | ||
| if (!(await exists(candidate))) return candidate; | ||
| } | ||
| return candidate; | ||
| } |
There was a problem hiding this comment.
uniquePath can overwrite an existing file when collisions exceed the fixed loop.
If date-slug.md through date-slug-99.md already exist, the function returns an existing path and writeFile overwrites it even when --force is not set.
Suggested fix
async function uniquePath(date, slug, force) {
let candidate = join(POSTS_DIR, `${date}-${slug}.md`);
if (force || !(await exists(candidate))) return candidate;
for (let n = 2; n < 100; n++) {
candidate = join(POSTS_DIR, `${date}-${slug}-${n}.md`);
if (!(await exists(candidate))) return candidate;
}
- return candidate;
+ throw new Error(
+ `No unique filename available for ${date}-${slug}.md (1..99 exhausted). Use --force to overwrite intentionally.`
+ );
}Also applies to: 133-133
🤖 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 `@scripts/generate.mjs` around lines 87 - 95, The uniquePath function can
return an existing file path when collisions for date-slug through date-slug-99
all exist, causing unintended overwrites. After the for loop (which iterates
from n=2 to n<100) completes without finding a unique path, the function
currently returns the last candidate value regardless of whether it exists. Fix
this by checking if the final candidate path exists after the loop completes—if
it does, either throw an error indicating too many collisions, or extend the
search range beyond 100 to continue looking for an available filename, ensuring
the function never returns a path that already exists unless the force flag is
explicitly set to true.
| let urlPath = decodeURIComponent((req.url || "/").split("?")[0]); | ||
| if (urlPath.endsWith("/")) urlPath += "index.html"; | ||
|
|
||
| const filePath = join(PUBLIC_DIR, urlPath); | ||
| // Prevent path traversal outside public/. | ||
| if (!filePath.startsWith(PUBLIC_DIR)) { | ||
| res.statusCode = 403; | ||
| return res.end("Forbidden"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node - <<'NODE'
const path = require("node:path");
const PUBLIC = "/tmp/public";
for (const u of ["/../publicity/probe.txt", "/../etc/passwd", "/index.html"]) {
const joined = path.join(PUBLIC, u);
const oldGuard = joined.startsWith(PUBLIC);
console.log({ u, joined, oldGuard });
}
NODERepository: xiami303/OpenBlog
Length of output: 291
🏁 Script executed:
# Check the file location and read the vulnerable code section
find . -name "serve.mjs" -type fRepository: xiami303/OpenBlog
Length of output: 81
🏁 Script executed:
# Read the serve.mjs file to verify the vulnerable code
head -50 scripts/serve.mjs | cat -nRepository: xiami303/OpenBlog
Length of output: 1921
🏁 Script executed:
# Test the suggested fix to ensure it properly prevents traversal
node - <<'NODE'
const path = require("node:path");
const PUBLIC = "/tmp/public";
// Test cases
const testCases = [
"/../publicity/probe.txt", // Sibling directory attack
"/../etc/passwd", // Absolute escape
"/index.html", // Normal file
"/subdir/file.txt", // Nested file
"//index.html", // Double slash
];
console.log("=== CURRENT (VULNERABLE) GUARD ===");
for (const u of testCases) {
const joined = path.join(PUBLIC, u);
const oldGuard = joined.startsWith(PUBLIC);
console.log({ u, path: joined, pass: oldGuard });
}
console.log("\n=== SUGGESTED FIX GUARD ===");
const sep = path.sep;
for (const u of testCases) {
try {
const filePath = path.resolve(PUBLIC, `.${u}`);
const newGuard = (filePath === PUBLIC || filePath.startsWith(PUBLIC + sep));
console.log({ u, path: filePath, pass: newGuard });
} catch (e) {
console.log({ u, error: e.message });
}
}
NODERepository: xiami303/OpenBlog
Length of output: 844
🏁 Script executed:
# Test decodeURIComponent exception handling
node - <<'NODE'
// Test if malformed percent-encoding throws
const testUrls = [
"/normal/path",
"/path%20with%20space",
"/malformed%ZZ", // Invalid percent encoding
"/path%", // Incomplete percent encoding
];
console.log("=== decodeURIComponent BEHAVIOR ===");
for (const url of testUrls) {
try {
const decoded = decodeURIComponent(url);
console.log({ url, decoded, error: null });
} catch (e) {
console.log({ url, error: e.message });
}
}
NODERepository: xiami303/OpenBlog
Length of output: 330
Harden path normalization and exception handling (current prefix check is bypassable and decodeURIComponent can throw).
The startsWith(PUBLIC_DIR) check allows prefix-confusion attacks—requests to a sibling directory like /publicity will pass the guard because /tmp/publicity starts with /tmp/public. Additionally, decodeURIComponent throws an exception on malformed percent-encoding (e.g., %ZZ or %), which will crash the request handler since it has no error handler.
Suggested fix
-import { join, extname, dirname } from "node:path";
+import { join, extname, dirname, resolve, sep } from "node:path";
const server = createServer(async (req, res) => {
- let urlPath = decodeURIComponent((req.url || "/").split("?")[0]);
+ let urlPath;
+ try {
+ urlPath = decodeURIComponent((req.url || "/").split("?")[0]);
+ } catch {
+ res.statusCode = 400;
+ return res.end("Bad request");
+ }
if (urlPath.endsWith("/")) urlPath += "index.html";
- const filePath = join(PUBLIC_DIR, urlPath);
+ const filePath = resolve(PUBLIC_DIR, `.${urlPath}`);
// Prevent path traversal outside public/.
- if (!filePath.startsWith(PUBLIC_DIR)) {
+ if (!(filePath === PUBLIC_DIR || filePath.startsWith(PUBLIC_DIR + sep))) {
res.statusCode = 403;
return res.end("Forbidden");
}📝 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.
| let urlPath = decodeURIComponent((req.url || "/").split("?")[0]); | |
| if (urlPath.endsWith("/")) urlPath += "index.html"; | |
| const filePath = join(PUBLIC_DIR, urlPath); | |
| // Prevent path traversal outside public/. | |
| if (!filePath.startsWith(PUBLIC_DIR)) { | |
| res.statusCode = 403; | |
| return res.end("Forbidden"); | |
| } | |
| import { join, extname, dirname, resolve, sep } from "node:path"; |
| let urlPath = decodeURIComponent((req.url || "/").split("?")[0]); | |
| if (urlPath.endsWith("/")) urlPath += "index.html"; | |
| const filePath = join(PUBLIC_DIR, urlPath); | |
| // Prevent path traversal outside public/. | |
| if (!filePath.startsWith(PUBLIC_DIR)) { | |
| res.statusCode = 403; | |
| return res.end("Forbidden"); | |
| } | |
| let urlPath; | |
| try { | |
| urlPath = decodeURIComponent((req.url || "/").split("?")[0]); | |
| } catch { | |
| res.statusCode = 400; | |
| return res.end("Bad request"); | |
| } | |
| if (urlPath.endsWith("/")) urlPath += "index.html"; | |
| const filePath = resolve(PUBLIC_DIR, `.${urlPath}`); | |
| // Prevent path traversal outside public/. | |
| if (!(filePath === PUBLIC_DIR || filePath.startsWith(PUBLIC_DIR + sep))) { | |
| res.statusCode = 403; | |
| return res.end("Forbidden"); | |
| } |
🤖 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 `@scripts/serve.mjs` around lines 27 - 35, The path validation has two
security/stability issues: the `filePath.startsWith(PUBLIC_DIR)` check is
vulnerable to prefix-confusion attacks (e.g., `/publicity` bypasses the guard)
and `decodeURIComponent` throws unhandled exceptions on malformed
percent-encoding. To fix this, wrap the `decodeURIComponent` call in a try-catch
block to respond with a 400 Bad Request on invalid input, and harden the path
check by using `resolve()` to normalize both `filePath` and `PUBLIC_DIR` before
comparing, ensuring the resolved filePath actually starts with the normalized
PUBLIC_DIR directory path rather than just matching the string prefix.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfce959f10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { data, content } = matter(raw); | ||
| const fileStat = await stat(full); | ||
| const noteDate = data.date ? new Date(data.date) : fileStat.mtime; | ||
| if (noteDate < cutoff) continue; |
There was a problem hiding this comment.
When --date is used for a backfill or when a note is accidentally future-dated, this check only drops notes older than the lower cutoff; anything after the target date still gets included. For example, generating --date 2026-06-10 with a memory/2026-06-12*.md note will publish content from after the requested day, so add an upper-bound check against the target date.
Useful? React with 👍 / 👎.
| const files = (await readdir(MEMORY_DIR)).filter( | ||
| (f) => extname(f) === ".md" && !f.startsWith(".") && f !== "README.md" | ||
| ); |
There was a problem hiding this comment.
Because this collector accepts every non-hidden memory/*.md except README.md, the committed memory/2026-06-12-example.md is treated as a real note. On scheduled/manual runs within seven days of 2026-06-12, a repo with no user notes and no API keys will still open a dry-run PR based on the sample text rather than doing nothing, so move examples outside memory/ or explicitly ignore example files.
Useful? React with 👍 / 👎.
|
@codifyai 来测试 |
1 participant
Summary by CodeRabbit
Release Notes
New Features
npm run generatecommand to create articles from memory notes locallyDocumentation