Skip to content

Commit 282c9a5

Browse files
Copilotpelikhan
Copilot
and
pelikhan
committed
revert: remove allowed-repos config from update_project, keep content-repo support only
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent ba434cb commit 282c9a5

File tree

7 files changed

+12
-306
lines changed

7 files changed

+12
-306
lines changed

‎actions/setup/js/update_project.cjs‎

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const { loadAgentOutput } = require("./load_agent_output.cjs");
55
const { getErrorMessage } = require("./error_helpers.cjs");
66
const { loadTemporaryIdMapFromResolved, resolveIssueNumber, isTemporaryId, normalizeTemporaryId } = require("./temporary_id.cjs");
77
const { logStagedPreviewInfo } = require("./staged_preview.cjs");
8-
const { ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_PERMISSION, ERR_VALIDATION } = require("./error_codes.cjs");
8+
const { ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_VALIDATION } = require("./error_codes.cjs");
99

1010
/**
1111
* Normalize agent output keys for update_project.
@@ -462,10 +462,9 @@ async function fetchAllProjectFields(github, projectId) {
462462
* @param {any} output - Safe output configuration
463463
* @param {Map<string, any>} temporaryIdMap - Map of temporary IDs to resolved issue numbers
464464
* @param {Object} githubClient - GitHub client (Octokit instance) to use for GraphQL queries
465-
* @param {Set<string>|null} allowedContentRepos - Optional set of allowed repos for content_repo validation. When non-empty, only repos in this set (plus the workflow's host repository from context.repo) are accepted as content_repo values. When null or empty, any repo is accepted.
466465
* @returns {Promise<void|{temporaryId?: string, draftItemId?: string}>} Returns undefined for most operations, or an object with temporary ID mapping for draft issue creation
467466
*/
468-
async function updateProject(output, temporaryIdMap = new Map(), githubClient = null, allowedContentRepos = null) {
467+
async function updateProject(output, temporaryIdMap = new Map(), githubClient = null) {
469468
output = normalizeUpdateProjectOutput(output);
470469

471470
// Use the provided github client, or fall back to the global github object
@@ -1020,20 +1019,6 @@ async function updateProject(output, temporaryIdMap = new Map(), githubClient =
10201019
const trimmedContentRepo = output.content_repo.trim();
10211020
const parts = trimmedContentRepo.split("/").map(p => p.trim());
10221021
if (parts.length === 2 && parts[0] && parts[1]) {
1023-
const requestedContentRepo = `${parts[0]}/${parts[1]}`;
1024-
const defaultRepo = `${owner}/${repo}`;
1025-
// Validate against allowed_repos when configured
1026-
if (allowedContentRepos !== null && allowedContentRepos.size > 0) {
1027-
const isDefaultRepo = requestedContentRepo === defaultRepo;
1028-
const isAllowed = isDefaultRepo || allowedContentRepos.has(requestedContentRepo);
1029-
if (!isAllowed) {
1030-
throw new Error(
1031-
`${ERR_PERMISSION}: content_repo "${requestedContentRepo}" is not in the allowed-repos list. ` +
1032-
`Allowed: ${defaultRepo}${allowedContentRepos.size > 0 ? ", " + Array.from(allowedContentRepos).join(", ") : ""}. ` +
1033-
`Configure allowed-repos under safe-outputs.update-project in your workflow frontmatter.`
1034-
);
1035-
}
1036-
}
10371022
contentOwner = parts[0];
10381023
contentRepo = parts[1];
10391024
core.info(`Using content_repo for resolution: ${contentOwner}/${contentRepo}`);
@@ -1246,12 +1231,6 @@ async function main(config = {}, githubClient = null) {
12461231
const configuredViews = Array.isArray(config.views) ? config.views : [];
12471232
const configuredFieldDefinitions = Array.isArray(config.field_definitions) ? config.field_definitions : [];
12481233

1249-
// Parse allowed-repos for content_repo validation (cross-repo issue/PR resolution)
1250-
// When non-empty, only repos in this set (plus the workflow's host repository) are accepted as content_repo values.
1251-
const rawAllowedRepos = Array.isArray(config.allowed_repos) ? config.allowed_repos : [];
1252-
const sanitizedAllowedRepos = rawAllowedRepos.map(r => (typeof r === "string" ? r.trim() : "")).filter(r => r);
1253-
const allowedContentRepos = new Set(sanitizedAllowedRepos);
1254-
12551234
// Check if we're in staged mode
12561235
const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true";
12571236

@@ -1261,9 +1240,6 @@ async function main(config = {}, githubClient = null) {
12611240
if (configuredFieldDefinitions.length > 0) {
12621241
core.info(`Found ${configuredFieldDefinitions.length} configured field definition(s) in frontmatter`);
12631242
}
1264-
if (allowedContentRepos.size > 0) {
1265-
core.info(`Allowed content repos: ${Array.from(allowedContentRepos).join(", ")}`);
1266-
}
12671243
core.info(`Max count: ${maxCount}`);
12681244

12691245
// Track state
@@ -1383,7 +1359,7 @@ async function main(config = {}, githubClient = null) {
13831359
};
13841360

13851361
try {
1386-
await updateProject(fieldsOutput, tempIdMap, github, allowedContentRepos);
1362+
await updateProject(fieldsOutput, tempIdMap, github);
13871363
core.info("✓ Created configured fields");
13881364
} catch (err) {
13891365
// prettier-ignore
@@ -1415,7 +1391,7 @@ async function main(config = {}, githubClient = null) {
14151391
}
14161392

14171393
// Process the update_project message
1418-
const updateResult = await updateProject(effectiveMessage, tempIdMap, github, allowedContentRepos);
1394+
const updateResult = await updateProject(effectiveMessage, tempIdMap, github);
14191395

14201396
// After processing the first message, create configured views if any
14211397
// Views are created after the first item is processed to ensure the project exists
@@ -1440,7 +1416,7 @@ async function main(config = {}, githubClient = null) {
14401416
},
14411417
};
14421418

1443-
await updateProject(viewOutput, tempIdMap, github, allowedContentRepos);
1419+
await updateProject(viewOutput, tempIdMap, github);
14441420
core.info(`✓ Created view ${i + 1}/${configuredViews.length}: ${viewConfig.name} (${viewConfig.layout})`);
14451421
} catch (err) {
14461422
// prettier-ignore

‎actions/setup/js/update_project.test.cjs‎

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -2128,111 +2128,3 @@ describe("update_project temporary project ID resolution", () => {
21282128
expect(mockCore.info).not.toHaveBeenCalledWith(expect.stringContaining("Resolved temporary project ID"));
21292129
});
21302130
});
2131-
2132-
describe("update_project allowed_repos for content_repo validation", () => {
2133-
let messageHandler;
2134-
2135-
beforeEach(async () => {
2136-
vi.clearAllMocks();
2137-
mockGithub.graphql.mockReset();
2138-
});
2139-
2140-
it("allows content_repo in the allowed list", async () => {
2141-
messageHandler = await updateProjectHandlerFactory({ allowed_repos: ["otherowner/otherrepo"] }, mockGithub);
2142-
const projectUrl = "https://github.com/orgs/testowner/projects/60";
2143-
2144-
queueResponses([repoResponse(), viewerResponse(), orgProjectV2Response(projectUrl, 60, "project-allowed"), issueResponse("allowed-issue-id"), emptyItemsResponse(), { addProjectV2ItemById: { item: { id: "allowed-item" } } }]);
2145-
2146-
const result = await messageHandler(
2147-
{
2148-
type: "update_project",
2149-
project: projectUrl,
2150-
content_type: "issue",
2151-
content_number: 42,
2152-
content_repo: "otherowner/otherrepo",
2153-
},
2154-
{},
2155-
new Map()
2156-
);
2157-
2158-
expect(result.success).toBe(true);
2159-
const contentQuery = mockGithub.graphql.mock.calls.find(([query, vars]) => query.includes("issue(number:") && vars?.owner === "otherowner");
2160-
expect(contentQuery).toBeDefined();
2161-
expect(contentQuery[1]).toMatchObject({ owner: "otherowner", repo: "otherrepo", number: 42 });
2162-
});
2163-
2164-
it("rejects content_repo not in the allowed list", async () => {
2165-
messageHandler = await updateProjectHandlerFactory({ allowed_repos: ["otherowner/otherrepo"] }, mockGithub);
2166-
const projectUrl = "https://github.com/orgs/testowner/projects/60";
2167-
2168-
queueResponses([repoResponse(), viewerResponse(), orgProjectV2Response(projectUrl, 60, "project-rejected")]);
2169-
2170-
const result = await messageHandler(
2171-
{
2172-
type: "update_project",
2173-
project: projectUrl,
2174-
content_type: "issue",
2175-
content_number: 42,
2176-
content_repo: "evilorg/evilrepo",
2177-
},
2178-
{},
2179-
new Map()
2180-
);
2181-
2182-
expect(result.success).toBe(false);
2183-
expect(result.error).toContain("evilorg/evilrepo");
2184-
expect(result.error).toContain("not in the allowed-repos list");
2185-
});
2186-
2187-
it("always allows context.repo as content_repo even when allowed_repos is configured", async () => {
2188-
messageHandler = await updateProjectHandlerFactory({ allowed_repos: ["someorg/other"] }, mockGithub);
2189-
const projectUrl = "https://github.com/orgs/testowner/projects/60";
2190-
2191-
queueResponses([repoResponse(), viewerResponse(), orgProjectV2Response(projectUrl, 60, "project-default"), issueResponse("default-issue-id"), emptyItemsResponse(), { addProjectV2ItemById: { item: { id: "default-item" } } }]);
2192-
2193-
// testowner/testrepo is context.repo - should always be allowed
2194-
const result = await messageHandler(
2195-
{
2196-
type: "update_project",
2197-
project: projectUrl,
2198-
content_type: "issue",
2199-
content_number: 7,
2200-
content_repo: "testowner/testrepo",
2201-
},
2202-
{},
2203-
new Map()
2204-
);
2205-
2206-
expect(result.success).toBe(true);
2207-
const contentQuery = mockGithub.graphql.mock.calls.find(([query, vars]) => query.includes("issue(number:") && vars?.owner === "testowner");
2208-
expect(contentQuery).toBeDefined();
2209-
});
2210-
2211-
it("allows any content_repo when allowed_repos is not configured", async () => {
2212-
messageHandler = await updateProjectHandlerFactory({}, mockGithub);
2213-
const projectUrl = "https://github.com/orgs/testowner/projects/60";
2214-
2215-
queueResponses([repoResponse(), viewerResponse(), orgProjectV2Response(projectUrl, 60, "project-open"), issueResponse("open-issue-id"), emptyItemsResponse(), { addProjectV2ItemById: { item: { id: "open-item" } } }]);
2216-
2217-
const result = await messageHandler(
2218-
{
2219-
type: "update_project",
2220-
project: projectUrl,
2221-
content_type: "issue",
2222-
content_number: 99,
2223-
content_repo: "anyorg/anyrepo",
2224-
},
2225-
{},
2226-
new Map()
2227-
);
2228-
2229-
expect(result.success).toBe(true);
2230-
expect(result.error).toBeUndefined();
2231-
});
2232-
2233-
it("logs allowed content repos at startup when configured", async () => {
2234-
messageHandler = await updateProjectHandlerFactory({ allowed_repos: ["org/repo1", "org/repo2"] }, mockGithub);
2235-
2236-
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Allowed content repos: org/repo1, org/repo2"));
2237-
});
2238-
});

‎pkg/cli/workflows/test-update-project-cross-repo.md‎

Lines changed: 0 additions & 63 deletions
This file was deleted.

‎pkg/parser/schemas/main_workflow_schema.json‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4691,13 +4691,6 @@
46914691
},
46924692
"additionalProperties": false
46934693
}
4694-
},
4695-
"allowed-repos": {
4696-
"type": "array",
4697-
"description": "Optional list of additional repositories (in 'owner/repo' format) that are allowed as content_repo values for cross-repo issue/PR resolution. When configured, agents may only reference issues or pull requests from these repositories (plus the workflow host repository). Wildcards are supported (e.g., 'myorg/*'). Example: ['github/docs', 'github/github'].",
4698-
"items": {
4699-
"type": "string"
4700-
}
47014694
}
47024695
},
47034696
"additionalProperties": false,
@@ -4708,11 +4701,6 @@
47084701
{
47094702
"github-token": "${{ secrets.PROJECT_GITHUB_TOKEN }}",
47104703
"max": 15
4711-
},
4712-
{
4713-
"allowed-repos": ["myorg/docs", "myorg/api"],
4714-
"github-token": "${{ secrets.PROJECT_GITHUB_TOKEN }}",
4715-
"max": 20
47164704
}
47174705
]
47184706
},

‎pkg/workflow/compiler_safe_outputs_config.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,8 +663,7 @@ var handlerRegistry = map[string]handlerBuilder{
663663
builder := newHandlerConfigBuilder().
664664
AddTemplatableInt("max", c.Max).
665665
AddIfNotEmpty("github-token", c.GitHubToken).
666-
AddIfNotEmpty("project", c.Project).
667-
AddStringSlice("allowed_repos", c.AllowedRepos)
666+
AddIfNotEmpty("project", c.Project)
668667
if len(c.Views) > 0 {
669668
builder.AddDefault("views", c.Views)
670669
}

‎pkg/workflow/safe_outputs_cross_repo_config_test.go‎

Lines changed: 3 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -420,91 +420,9 @@ func TestPushToPullRequestBranchCrossRepoInHandlerConfig(t *testing.T) {
420420
assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo")
421421
}
422422

423-
// TestUpdateProjectAllowedReposConfig verifies that the `allowed-repos` key in the
424-
// `update-project` frontmatter configuration is correctly parsed and populates the
425-
// AllowedRepos field on UpdateProjectConfig.
426-
func TestUpdateProjectAllowedReposConfig(t *testing.T) {
427-
compiler := NewCompiler()
428-
429-
tests := []struct {
430-
name string
431-
configMap map[string]any
432-
expectedRepos []string
433-
}{
434-
{
435-
name: "allowed-repos configured",
436-
configMap: map[string]any{
437-
"update-project": map[string]any{
438-
"allowed-repos": []any{"github/docs", "github/github"},
439-
},
440-
},
441-
expectedRepos: []string{"github/docs", "github/github"},
442-
},
443-
{
444-
name: "allowed-repos with single repo",
445-
configMap: map[string]any{
446-
"update-project": map[string]any{
447-
"allowed-repos": []any{"myorg/myrepo"},
448-
},
449-
},
450-
expectedRepos: []string{"myorg/myrepo"},
451-
},
452-
{
453-
name: "no allowed-repos",
454-
configMap: map[string]any{
455-
"update-project": map[string]any{
456-
"max": 5,
457-
},
458-
},
459-
expectedRepos: nil,
460-
},
461-
}
462-
463-
for _, tt := range tests {
464-
t.Run(tt.name, func(t *testing.T) {
465-
cfg := compiler.parseUpdateProjectConfig(tt.configMap)
466-
467-
require.NotNil(t, cfg, "config should not be nil")
468-
assert.Equal(t, tt.expectedRepos, cfg.AllowedRepos, "AllowedRepos mismatch")
469-
})
470-
}
471-
}
472-
473-
// TestUpdateProjectAllowedReposInHandlerConfig verifies the end-to-end flow: that
474-
// allowed-repos on UpdateProjectConfig is compiled into the GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG
475-
// environment variable as `allowed_repos`, so the JS runtime receives the configuration.
476-
func TestUpdateProjectAllowedReposInHandlerConfig(t *testing.T) {
477-
compiler := NewCompiler()
478-
479-
workflowData := &WorkflowData{
480-
Name: "Test",
481-
SafeOutputs: &SafeOutputsConfig{
482-
UpdateProjects: &UpdateProjectConfig{
483-
GitHubToken: "${{ secrets.PROJECT_GITHUB_TOKEN }}",
484-
Project: "https://github.com/orgs/myorg/projects/42",
485-
AllowedRepos: []string{"github/docs", "github/github"},
486-
},
487-
},
488-
}
489-
490-
var steps []string
491-
compiler.addHandlerManagerConfigEnvVar(&steps, workflowData)
492-
493-
require.NotEmpty(t, steps)
494-
handlerConfig := extractHandlerConfig(t, strings.Join(steps, ""))
495-
496-
updateProject, ok := handlerConfig["update_project"]
497-
require.True(t, ok, "update_project config should be present")
498-
499-
assert.Equal(t, "${{ secrets.PROJECT_GITHUB_TOKEN }}", updateProject["github-token"], "github-token should be in handler config")
500-
assert.Equal(t, "https://github.com/orgs/myorg/projects/42", updateProject["project"], "project should be in handler config")
501-
502-
allowedRepos, ok := updateProject["allowed_repos"]
503-
require.True(t, ok, "allowed_repos should be present")
504-
assert.Contains(t, allowedRepos, "github/docs", "allowed_repos should contain github/docs")
505-
assert.Contains(t, allowedRepos, "github/github", "allowed_repos should contain github/github")
506-
}
507-
423+
// TestHandlerManagerStepPerOutputTokenInHandlerConfig verifies that per-output tokens
424+
// (e.g., add-comment.github-token) are wired into the handler config JSON (GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG)
425+
// but NOT used as the step-level with.github-token. The step-level token follows the same
508426
// precedence as github_token.go: project token > global safe-outputs token > magic secrets.
509427
func TestHandlerManagerStepPerOutputTokenInHandlerConfig(t *testing.T) {
510428
compiler := NewCompiler()

0 commit comments

Comments
 (0)