Conversation
Automated sync from stranske/Workflows Template hash: b653eb470804 Changes synced from sync-manifest.yml
stranske
added
sync
📝 WalkthroughWalkthroughTwo ChangesScript: repo validation and orchestrator skill context path support
CI: Claude Code Review workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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 |
GitHub Actions
Failure
|
Workflow state fingerprint for Keepalive Loop Reporter. Do not edit. |
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
![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
🤖 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/maint-76-claude-code-review.yml:
- Line 192: The inline version comment in the anthropics/claude-code-action
action reference is inaccurate. Update the comment from `# v1` to `# v1.0.153`
to correctly reflect the actual version tag that corresponds to the specified
commit hash. This ensures the comment accurately documents which version of the
action is being used.
In `@scripts/runner_lib/core.py`:
- Around line 426-431: The `orchestrator_summary_path` currently allows
arbitrary absolute paths to be used when sourced from the environment variable
at line 955, which poses a security risk since the file contents are included in
prompt output at line 465. Modify the path handling logic in the section
starting with the `orchestrator_summary_path` assignment to ensure that the
final resolved path is always constrained within the workspace directory
boundary, regardless of whether it was specified as an absolute or relative
path. Validate that the resolved path is a child of the workspace using path
resolution and comparison methods to prevent arbitrary file inclusion from
untrusted environment inputs.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f70dddfd-e1ef-430c-bdd5-8c01c03d8b8b
📒 Files selected for processing (4)
.github/workflows/maint-76-claude-code-review.ymlscripts/orchestrator_skill.pyscripts/reference_packs.pyscripts/runner_lib/core.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Workflows(auto-detected)stranske/Template(auto-detected)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For Manager-Database repository: use Prefect 2.x and import schedules from
prefect.client.schemas.schedules
Files:
scripts/reference_packs.pyscripts/orchestrator_skill.pyscripts/runner_lib/core.py
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
startup_failurein GitHub Actions workflows with zero jobs indicates GitHub couldn't parse the workflow; check for invalid YAML syntax, conflictingpermissions:blocks onworkflow_callreusable workflows, invalid permission scopes, or circular workflow references
Files:
.github/workflows/maint-76-claude-code-review.yml
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Reference reusable workflows with
@mainby default unless intentionally pinning to an exact commit SHA for a documented reason
Files:
.github/workflows/maint-76-claude-code-review.yml
**/.github/workflows/*.yml
📄 CodeRabbit inference engine (AGENTS.md)
Reference reusable workflows with
@mainunless intentionally pinning to an exact commit SHA for a controlled reason.
Files:
.github/workflows/maint-76-claude-code-review.yml
🔀 Multi-repo context stranske/Workflows, stranske/Template
Linked repositories findings
stranske/Workflows [::stranske/Workflows::]
Repository validation change impact
The PR introduces stricter validation of the repo field in both scripts/reference_packs.py and scripts/orchestrator_skill.py. The new _validate_repo function enforces that the repository string must contain exactly two non-empty segments when split by / (format: owner/name).
Breaking validation pattern:
scripts/reference_packs.py:_validate_repo()[::stranske/Workflows::] — Changed to reject repo strings that don't match exactlyowner/nameformat (e.g., rejectstrend/research/extra,a//b,/a,a/)scripts/orchestrator_skill.py:_validate_repo()[::stranske/Workflows::] — Same stricter validation applied
Test evidence:
tests/scripts/test_reference_packs.py:test_parse_reference_packs_rejects_nested_repo_names()[::stranske/Workflows::] — Explicitly validates that"repo": "trend/research/extra"is rejected with error"repo must use owner/name format"tests/scripts/test_orchestrator_skill.py:test_parse_rejects_nested_repo_names()[::stranske/Workflows::] — Same validation rejection for"repo": "owner/repo/extra"
Configuration files affected
Both .github/reference_packs.json and .github/orchestrator_skill.json are optional configuration files where the repo field is validated:
docs/ci/ORCHESTRATOR_SKILL_CONTEXT.md[::stranske/Workflows::] — Documents the configuration format and requires repos in format like"stranske/Workflows"and"trend/research"scripts/reference_packs.py:parse_reference_packs()[::stranske/Workflows::] — Validatesrepofield in pack definitionsscripts/orchestrator_skill.py:parse_orchestrator_skill_config()[::stranske/Workflows::] — Validates inlinerepofield in orchestrator skill config
Runner context changes
scripts/runner_lib/core.py:assemble_prompt() [::stranske/Workflows::] — Now supports conditional materialization of orchestrator skill context:
- If
materialize_orchestrator_skillis enabled, it callsmaterialize_orchestrator_skill()and captures the returned summary path - Otherwise, reads
orchestrator_skill_summary_pathfrom context and resolves it relative to workspace when not absolute - Only includes "## Orchestrator Skill Context" section if the resolved summary file exists
stranske/Template [::stranske/Template::]
No actual configuration files found — Neither .github/orchestrator_skill.json nor .github/reference_packs.json exist in the Template repository outside of test fixtures, indicating Template itself does not currently consume these features. The synced scripts are present but not actively used.
Breaking change summary
Any consumer repository using .github/reference_packs.json or .github/orchestrator_skill.json with repo fields containing:
- Multiple slashes (e.g.,
owner/org/repo) - Empty segments (e.g.,
owner//repoor/owner/repo) - Trailing/leading slashes
...will now fail validation during workflow execution with error: "repo must use owner/name format".
🔇 Additional comments (3)
scripts/reference_packs.py (1)
86-87: LGTM!scripts/runner_lib/core.py (1)
8-8: LGTM!Also applies to: 370-376, 419-424, 461-462
scripts/orchestrator_skill.py (1)
83-84: Validation code is correct; no config files exist in this repository to validate.The stricter
owner/namevalidation at lines 83-84 properly enforces the contract (splits on "/" and requires exactly 2 non-empty parts). However, Fine-Art-Archive has no.github/orchestrator_skill.jsonor.github/reference_packs.jsonfiles, so the validation will never be triggered at runtime. The code gracefully handles missing configs by returningNone, preventing any failures.
| id: claude | ||
| continue-on-error: true | ||
| uses: anthropics/claude-code-action@51705da45eecce209d4700538bf8377d5b5fc695 # v1 | ||
| uses: anthropics/claude-code-action@2fee15510437d71399d9139ed60433470484a8fb # v1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the anthropics/claude-code-action commit hash and version
# Check if the commit exists and get its details
echo "=== Checking commit details ==="
gh api repos/anthropics/claude-code-action/commits/2fee15510437d71399d9139ed60433470484a8fb \
--jq '{sha: .sha, author: .commit.author.name, date: .commit.author.date, message: .commit.message}' 2>&1 || echo "Commit not found or API error"
echo ""
echo "=== Checking tags containing this commit ==="
gh api repos/anthropics/claude-code-action/tags --jq '.[] | select(.commit.sha == "2fee15510437d71399d9139ed60433470484a8fb") | {name: .name, sha: .commit.sha}' 2>&1 || echo "No tags found or API error"
echo ""
echo "=== Checking for security advisories ==="
gh api graphql -f query='
{
repository(owner: "anthropics", name: "claude-code-action") {
vulnerabilityAlerts(first: 5) {
nodes {
securityVulnerability {
advisory {
summary
severity
publishedAt
}
}
}
}
}
}' 2>&1 || echo "Unable to query advisories or none found"Repository: stranske/Fine-Art-Archive
Length of output: 497
Update version comment to reflect actual tag: use # v1.0.153 instead of # v1.
The commit hash 2fee15510437d71399d9139ed60433470484a8fb is valid and corresponds to tag v1.0.153 in the anthropics/claude-code-action repository. No security advisories exist for this version. However, the inline comment should be corrected to match the actual version tag for accuracy.
🤖 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/maint-76-claude-code-review.yml at line 192, The inline
version comment in the anthropics/claude-code-action action reference is
inaccurate. Update the comment from `# v1` to `# v1.0.153` to correctly reflect
the actual version tag that corresponds to the specified commit hash. This
ensures the comment accurately documents which version of the action is being
used.
| orchestrator_summary_raw = context.get("orchestrator_skill_summary_path") | ||
| orchestrator_summary_path = ( | ||
| Path(str(orchestrator_summary_raw)) if orchestrator_summary_raw else None | ||
| ) | ||
| if orchestrator_summary_path and not orchestrator_summary_path.is_absolute(): | ||
| orchestrator_summary_path = workspace / orchestrator_summary_path |
There was a problem hiding this comment.
Constrain orchestrator_skill_summary_path to the workspace boundary.
Line 955 sources the path from env, and Line 426-431 allows absolute paths; Line 465 then reads that file into prompt output. This permits arbitrary file inclusion if the env value is influenced.
Proposed fix
@@
else:
orchestrator_summary_raw = context.get("orchestrator_skill_summary_path")
orchestrator_summary_path = (
Path(str(orchestrator_summary_raw)) if orchestrator_summary_raw else None
)
- if orchestrator_summary_path and not orchestrator_summary_path.is_absolute():
- orchestrator_summary_path = workspace / orchestrator_summary_path
+ if orchestrator_summary_path:
+ if not orchestrator_summary_path.is_absolute():
+ orchestrator_summary_path = workspace / orchestrator_summary_path
+ orchestrator_summary_path = orchestrator_summary_path.resolve()
+ workspace_root = workspace.resolve()
+ try:
+ orchestrator_summary_path.relative_to(workspace_root)
+ except ValueError as exc:
+ raise ValueError(
+ "orchestrator_skill_summary_path must resolve within workspace"
+ ) from excAlso applies to: 461-465, 955-955
🤖 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/runner_lib/core.py` around lines 426 - 431, The
`orchestrator_summary_path` currently allows arbitrary absolute paths to be used
when sourced from the environment variable at line 955, which poses a security
risk since the file contents are included in prompt output at line 465. Modify
the path handling logic in the section starting with the
`orchestrator_summary_path` assignment to ensure that the final resolved path is
always constrained within the workspace directory boundary, regardless of
whether it was specified as an absolute or relative path. Validate that the
resolved path is a child of the workspace using path resolution and comparison
methods to prevent arbitrary file inclusion from untrusted environment inputs.
|
Closing as stale: newer replacement #119 exists from sync/workflows-76689bc445fd. |
This branch was previously deployed
1 participant
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
1bc0f231da20f5596199ce25d3923f9230ac4b76Template hash:
b653eb470804Sync branch:
sync/workflows-b653eb470804Consumer repo:
stranske/Fine-Art-ArchiveManifest:
.github/sync-manifest.ymlSummary by CodeRabbit
New Features
Bug Fixes
owner/nameformat, rejecting previously accepted invalid patterns.Chores