Skip to content

fix(registry): reject unsafe content slugs#2093

Open
JSONbored wants to merge 1 commit into
mainfrom
codex/propose-fix-for-content-pr-vulnerability
Open

fix(registry): reject unsafe content slugs#2093
JSONbored wants to merge 1 commit into
mainfrom
codex/propose-fix-for-content-pr-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Close a security gap where content-only MDX PRs could pass the content gate while allowing a malicious slug (e.g. ../../../../foo) to cause generated artifact writes outside apps/web/public/data when the registry builder runs after merge.
  • Ensure the content validator and artifact generator enforce a safe slug format and that artifact writes are contained under the public data directory.

Description

  • Add a shared SAFE_CONTENT_SLUG_PATTERN and reject frontmatter slugs that do not match the safe pattern in validateEntry so traversal-style slugs fail content validation (packages/registry/src/content-schema.js).
  • Fail artifact generation early for unsafe slugs by validating entry.slug in the registry artifact builder (packages/registry/src/artifacts.js).
  • Add a path containment guard artifactOutputPath() that resolves artifact target paths and refuses to write files whose resolved path is outside apps/web/public/data, and use it when writing generated artifacts (scripts/build-content-index.mjs).
  • Add regression tests that validate both the content validator and artifact builder reject traversal-style slugs, and add an integration-style validation fixture to tests/content-validation.test.ts (tests/artifact-path-safety.test.ts, updated tests/content-validation.test.ts).

Testing

  • Ran pnpm exec vitest run tests/artifact-path-safety.test.ts tests/content-validation.test.ts tests/classify-pr-changes.test.ts --reporter=dot and all included tests passed.
  • Ran pnpm validate:content:strict (which invokes node scripts/validate-content.mjs --strict-recommended) and it completed successfully (Content validation passed.).
  • Ran node scripts/build-content-index.mjs to exercise artifact generation and confirmed artifact writes are blocked for unsafe slugs and normal generation succeeds for existing content.
  • Ran pnpm validate:packages and package-level validations passed.
  • Ran pnpm test:registry-artifacts; 43/45 tests passed while 2 long-running artifact tests hit the per-test timeout in this environment (timeouts observed during this run).

Codex Task

@gittensory

gittensory Bot commented Jun 11, 2026

Copy link
Copy Markdown

Important

Gittensory found maintainer review notes

Public GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed.

Readiness score: 93/100

Signal Result Evidence Action
Linked issue ⚠️ Missing No linked issue or no-issue rationale found. Explain no-issue PR.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found. No action.
Review load ✅ 20/20 Readiness component derived from cached public PR metadata and labels; size label size:M. No action.
Validation evidence ✅ 25/25 PR body includes validation/test evidence. No action.
Open PR queue ✅ 10/10 0 open PR(s), 0 likely reviewable. No action.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 62 PR(s), 347 issue(s). No action.
Gate result ✅ Passing No configured blocker found. No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 62 PR(s), 347 issue(s).
  • PR-specific overlap: none found.
Maintainer notes
  • No linked issue detected: The planned PR does not reference a closing issue or explicit linked issue number.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Re-run Gittensory review

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. Learn more about Gittensor contribution workflows.

@gittensory gittensory Bot added the gittensory:reviewed Gittensor contributor context label Jun 11, 2026
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 11, 2026
@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@JSONbored, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 6 minutes and 44 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 81494b6c-3bfa-4644-84ba-860ece39d466

📥 Commits

Reviewing files that changed from the base of the PR and between ec30bc5 and b287a27.

📒 Files selected for processing (5)
  • packages/registry/src/artifacts.js
  • packages/registry/src/content-schema.js
  • scripts/build-content-index.mjs
  • tests/artifact-path-safety.test.ts
  • tests/content-validation.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/propose-fix-for-content-pr-vulnerability

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@JSONbored JSONbored self-assigned this Jun 11, 2026
@JSONbored JSONbored closed this Jun 11, 2026
@JSONbored JSONbored reopened this Jun 11, 2026
@reviewwed

reviewwed Bot commented Jun 11, 2026

Copy link
Copy Markdown

🔍 Maintainer advisory review

Reviewed 5 changed file(s) — two independent AI reviewers. This is an advisory review — it does not merge or close the PR.

Suggested action: 🛠️ Request changes. Address the items below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
The change adds important safety checks for content slugs, preventing path traversal both at validation time and when writing artifact files. The new SAFE_CONTENT_SLUG_PATTERN and artifactOutputPath guard against directory escapes, which is a solid security improvement. However, the regex is very restrictive (lowercase alphanumeric and single hyphens only). If any existing entries in the repository use uppercase letters, underscores, or other allowed characters, the build/validation pipeline will now fail, causing a regression. The PR does not address migration of existing data or provide a way to relax the rule for legacy content. Apart from that, the implementation is correct, tests cover the new logic, and the code follows existing patterns.

Risks:

  • Potential breakage of the content build because existing slugs may not match the new pattern.
  • Duplicate slug validation (both in validateEntry and buildRegistryArtifactSet) could be consolidated.
  • No explicit migration path for already‑published content.

Suggestions:

  1. Add a migration step or documentation to update existing slugs to the new format, or relax the regex to also allow characters currently used in the dataset (e.g., underscores, uppercase).
  2. Consider centralising slug validation in a single helper to avoid duplication.
  3. Add a test that runs the full content build on a sample of real repository data to ensure no unexpected failures.
  4. Ensure the artifactOutputPath function is exported and unit‑tested (optional but improves confidence).

Suggestions

  • Adjust SAFE_CONTENT_SLUG_PATTERN to allow characters present in existing slugs or provide a migration script.
  • Consolidate slug validation into a shared utility to avoid duplicate checks.
  • Add integration test that validates the entire content set with the new rules.
  • Document the new slug requirements in the contribution guidelines.

Worth double-checking

  • Build failures due to legacy slugs not matching the new strict pattern.
  • Potential disruption of the public site if existing entries are rejected.

Reviewer B · nemotron-3-120b-a12b — recommends 🛠️ request changes
The change adds a strict slug pattern (lowercase alphanumerics and single hyphens) and validates it both in content-schema validation and during artifact building, with an additional path‑safety check for file writes. New tests verify that unsafe slugs are rejected. This addresses path‑traversal risks and follows the repo's patterns, but may affect existing entries that do not conform to the new pattern.

Suggestions

  • Run the content validation on the current registry to confirm no existing entries violate the new slug pattern (e.g., no uppercase letters, underscores, or leading/trailing hyphens).
  • Update the contribution guide to document the required slug format.
  • If any current entries fail, either update their slugs or assess whether a migration/grandfathering approach is needed.

Worth double-checking

  • Existing entries with uppercase letters, underscores, or leading/trailing hyphens in their slugs would now fail validation and break the build.
  • If any entry type permits an empty slug, the new validation in buildRegistryArtifactSet would reject it (empty string does not match the pattern).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensory:reviewed Gittensor contributor context size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant