Repository navigation
ci(e2e): use commit SHA for upstream workflow refs in e2e tests #2094
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package cli | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/fullsend-ai/fullsend/internal/forge" | ||
| "github.com/fullsend-ai/fullsend/internal/ui" | ||
| ) | ||
|
|
||
| // pinUpstreamRef rewrites @v0 references in scaffolded workflow files to | ||
| // point to the given git ref. This is committed as a separate commit after | ||
| // install so the history shows the original v0 install followed by an | ||
| // explicit pin. Users can use this to pin to a release tag (e.g. v0.15.0) | ||
| // or a specific commit SHA for testing. | ||
| func pinUpstreamRef(ctx context.Context, client forge.Client, printer *ui.Printer, owner, repo, ref string, workflowPaths []string) error { | ||
| printer.StepStart(fmt.Sprintf("Pinning upstream ref to %s", ref)) | ||
|
|
||
| var updated []forge.TreeFile | ||
| for _, path := range workflowPaths { | ||
| content, err := client.GetFileContent(ctx, owner, repo, path) | ||
| if err != nil { | ||
| if forge.IsNotFound(err) { | ||
| continue | ||
| } | ||
| return fmt.Errorf("reading %s: %w", path, err) | ||
| } | ||
| rewritten := bytes.ReplaceAll(content, []byte("@v0"), []byte("@"+ref)) | ||
| rewritten = bytes.ReplaceAll(rewritten, []byte("fullsend_ai_ref: v0"), []byte("fullsend_ai_ref: "+ref)) | ||
| rewritten = bytes.ReplaceAll(rewritten, []byte("ref: v0"), []byte("ref: "+ref)) | ||
| if !bytes.Equal(content, rewritten) { | ||
| updated = append(updated, forge.TreeFile{ | ||
| Path: path, | ||
|
There was a problem hiding this comment. [low] edge-case The ref: v0 replacement pattern is generic YAML and could match unrelated lines if a future workflow file in perOrgWorkflowPaths contains ref: v0 in a non-checkout context. Risk mitigated by hardcoded paths to scaffold-generated files. Suggested fix: Consider using a more specific pattern such as indentation-scoped match. |
||
| Content: rewritten, | ||
| Mode: "100644", | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| if len(updated) == 0 { | ||
| printer.StepDone("No workflow files needed pinning") | ||
| return nil | ||
| } | ||
|
|
||
| committed, err := client.CommitFiles(ctx, owner, repo, | ||
| fmt.Sprintf("chore: pin upstream ref to %s", ref), | ||
| updated) | ||
| if err != nil { | ||
| printer.StepFail("Failed to pin upstream ref") | ||
| return fmt.Errorf("committing upstream ref pin: %w", err) | ||
| } | ||
| if committed { | ||
| printer.StepDone(fmt.Sprintf("Pinned %d workflow files to %s", len(updated), ref)) | ||
| } else { | ||
| printer.StepDone("Workflow files already pinned") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // perOrgWorkflowPaths lists the scaffold workflow files installed in | ||
|
There was a problem hiding this comment. [low] comment-style Package-level variable comments for perOrgWorkflowPaths and perRepoWorkflowPaths use verb form instead of noun-phrase form. |
||
| // the per-org config repo (.fullsend). | ||
| var perOrgWorkflowPaths = []string{ | ||
| ".github/workflows/code.yml", | ||
| ".github/workflows/fix.yml", | ||
| ".github/workflows/prioritize-scheduler.yml", | ||
| ".github/workflows/prioritize.yml", | ||
| ".github/workflows/repo-maintenance.yml", | ||
| ".github/workflows/retro.yml", | ||
| ".github/workflows/review.yml", | ||
| ".github/workflows/triage.yml", | ||
| } | ||
|
|
||
| // perRepoWorkflowPaths lists the workflow files installed in per-repo mode. | ||
| var perRepoWorkflowPaths = []string{ | ||
| ".github/workflows/fullsend.yaml", | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package cli | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/fullsend-ai/fullsend/internal/forge" | ||
| "github.com/fullsend-ai/fullsend/internal/ui" | ||
| ) | ||
|
|
||
| func TestPinUpstreamRef(t *testing.T) { | ||
| ctx := context.Background() | ||
|
|
||
| t.Run("rewrites all v0 patterns", func(t *testing.T) { | ||
|
There was a problem hiding this comment. [low] test-adequacy The test suite does not cover the per-repo workflow path (perRepoWorkflowPaths / .github/workflows/fullsend.yaml). Only per-org patterns are tested. |
||
| fc := forge.NewFakeClient() | ||
| fc.FileContents["org/repo/.github/workflows/triage.yml"] = | ||
| []byte(" uses: fullsend-ai/fullsend/.github/workflows/reusable-triage.yml@v0\n" + | ||
| " fullsend_ai_ref: v0\n") | ||
| fc.FileContents["org/repo/.github/workflows/repo-maintenance.yml"] = | ||
| []byte(" ref: v0\n" + | ||
| " uses: fullsend-ai/fullsend/.github/actions/mint-token@v0\n") | ||
|
|
||
| var buf bytes.Buffer | ||
| printer := ui.New(&buf) | ||
| err := pinUpstreamRef(ctx, fc, printer, "org", "repo", "abc123def456", | ||
| []string{".github/workflows/triage.yml", ".github/workflows/repo-maintenance.yml"}) | ||
| require.NoError(t, err) | ||
|
|
||
| triage, err := fc.GetFileContent(ctx, "org", "repo", ".github/workflows/triage.yml") | ||
| require.NoError(t, err) | ||
| assert.Contains(t, string(triage), "@abc123def456") | ||
| assert.Contains(t, string(triage), "fullsend_ai_ref: abc123def456") | ||
| assert.NotContains(t, string(triage), "@v0") | ||
|
|
||
| maint, err := fc.GetFileContent(ctx, "org", "repo", ".github/workflows/repo-maintenance.yml") | ||
| require.NoError(t, err) | ||
| assert.Contains(t, string(maint), "ref: abc123def456") | ||
| assert.Contains(t, string(maint), "@abc123def456") | ||
| assert.NotContains(t, string(maint), "@v0") | ||
| }) | ||
|
|
||
| t.Run("skips missing files", func(t *testing.T) { | ||
| fc := forge.NewFakeClient() | ||
| var buf bytes.Buffer | ||
| printer := ui.New(&buf) | ||
| err := pinUpstreamRef(ctx, fc, printer, "org", "repo", "abc123def456", | ||
| []string{".github/workflows/nonexistent.yml"}) | ||
| require.NoError(t, err) | ||
| }) | ||
|
|
||
| t.Run("no-op when files have no v0 refs", func(t *testing.T) { | ||
| fc := forge.NewFakeClient() | ||
| fc.FileContents["org/repo/.github/workflows/custom.yml"] = | ||
| []byte("uses: some-other/action@v1\n") | ||
| var buf bytes.Buffer | ||
| printer := ui.New(&buf) | ||
| err := pinUpstreamRef(ctx, fc, printer, "org", "repo", "abc123def456", | ||
| []string{".github/workflows/custom.yml"}) | ||
| require.NoError(t, err) | ||
| // File should be unchanged | ||
| content, _ := fc.GetFileContent(ctx, "org", "repo", ".github/workflows/custom.yml") | ||
| assert.Equal(t, "uses: some-other/action@v1\n", string(content)) | ||
| }) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
[low] injection
pinUpstreamRef performs bytes.ReplaceAll substitution of a user-supplied ref value into workflow YAML content with no input validation. Attack surface limited to org admins; automated caller passes GitHub-provided commit SHA.
Suggested fix: Validate ref matches ^[a-zA-Z0-9._/-]+$ before substitution.