Skip to content

remove sprintf for prefixing scopes #519

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.

Sign up for GitHub

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

Open
smarcet wants to merge 1 commit into
base: main
Choose a base branch
from
+1,774 −1,664
Open

remove sprintf for prefixing scopes #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions .github/workflows/l5-swagger-generate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,99 @@ jobs:
- name: Generate OpenAPI docs
run: php artisan l5-swagger:generate

- name: Build Swagger UI preview
run: |
mkdir -p swagger-ui
cp storage/api-docs/api-docs.json swagger-ui/api-docs.json
cat > swagger-ui/index.html << 'HTMLEOF'
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>OpenStackID API - Swagger UI</title>
<link rel="stylesheet" href="https://unpkg.com/swagger-ui-dist/swagger-ui.css" />
</head>
<body>
<div id="swagger-ui"></div>
<script src="https://unpkg.com/swagger-ui-dist/swagger-ui-bundle.js"></script>
Comment on lines +125 to +129
<script>
window.onload = () => {
window.ui = SwaggerUIBundle({
url: 'api-docs.json',
dom_id: '#swagger-ui'
});
};
</script>
</body>
</html>
HTMLEOF

- name: Upload Swagger UI artifact
uses: actions/upload-artifact@v4
with:
name: swagger-ui
path: swagger-ui/
if-no-files-found: error

pages-preview:
name: Publish Swagger UI to GitHub Pages
needs: openapi-generate
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest

permissions:
contents: write
issues: write
pull-requests: write

Comment on lines +149 to +159
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Download Swagger UI artifact
uses: actions/download-artifact@v4
with:
name: swagger-ui
path: swagger-ui

- name: Build public directory for GitHub Pages
run: |
PR_PATH="openapi/pr-${{ github.event.number }}"
mkdir -p "public/${PR_PATH}"
cp -R swagger-ui/* "public/${PR_PATH}/"
echo "Built GitHub Pages content at public/${PR_PATH}"

- name: Publish to GitHub Pages
uses: peaceiris/actions-gh-pages@v4
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: ./public
keep_files: true

Comment on lines +149 to +183
Copy link

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

GitHub Actions GITHUB_TOKEN fork pull request read-only permissions

💡 Result:

For workflows triggered by pull_request events from a fork, GitHub deliberately downgrades GITHUB_TOKEN to read-only and does not pass repository secrets (this is not overrideable via permissions: in the workflow). [1][2]

Key implications / options:

  • You can’t “grant write” to GITHUB_TOKEN for fork PR runs on pull_request; GitHub caps it at read-only for safety. [1][2]
  • If you need privileged automation on PRs (labeling, commenting, etc.), many repos use pull_request_target, which runs in the base repo context and therefore can have read/write GITHUB_TOKEN and access to secrets unless you restrict it—but this is security-sensitive (don’t run untrusted fork code in that context). [1]
  • For private/internal repos, there are repository/enterprise settings to allow fork PR workflows and (optionally) “send write tokens” and/or “send secrets” to fork PR workflows. [3]

Sources: GitHub Actions docs on fork PR restrictions and pull_request_target behavior [1], GitHub changelog noting fork PR tokens are always read-only [2], and GitHub settings/policy docs for fork PR workflows in private repos (including “send write tokens”) [3].

References:
[1] (docs.github.com)
[2] (github.blog)
[3] (docs.github.com)

Citations:

🌐 Web query:

peaceiris/actions-gh-pages write permissions requirements

💡 Result:

To use peaceiris/actions-gh-pages with GITHUB_TOKEN, your workflow/job must grant the token write access to repository contents (so it can push commits/tags to the publishing branch, usually gh-pages). The README explicitly calls out fixing 403 Write access to repository not granted by adding: permissions: contents: write. (github.com)

Example:

jobs:
  deploy:
    permissions:
      contents: write
    steps:
      - uses: peaceiris/actions-gh-pages@v4
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          publish_dir: ./public

(github.com)

If you still get “permission denied” errors, also check the repository setting Settings → Actions → General → Workflow permissions and ensure it’s not restricted to read-only (many repos/orgs default to read-only). (stackoverflow.com)

If you deploy to an external repository (external_repository:), GITHUB_TOKEN won’t work; you must use a deploy key or a personal access token instead. (github.com)

Citations:

Guard the deploy job for same-repo PRs.

pull_request runs from forks receive a read-only GITHUB_TOKEN, which peaceiris/actions-gh-pages@v4 cannot use to push to the repository. This job will fail for external contributors. Add a guard condition to prevent execution on fork PRs:

-        if: github.event_name == 'pull_request'
+        if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository

If fork previews are required, use pull_request_target in a separate, trusted workflow instead.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/l5-swagger-generate.yml around lines 149 - 183, The
pages-preview job runs on pull_request events but will fail for forked PRs
because the read-only GITHUB_TOKEN prevents peaceiris/actions-gh-pages@v4 from
pushing; update the job's if condition (job name: pages-preview) to only run for
same-repo PRs by adding a fork check, e.g. change the existing if:
github.event_name == 'pull_request' to include
github.event.pull_request.head.repo.fork == false (or equivalent comparison such
as github.event.pull_request.head.repo.full_name == github.repository) so the
peaceiris/actions-gh-pages@v4 step only executes for non-fork PRs.

- name: Comment preview URL on PR
uses: actions/github-script@v7
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const prNumber = context.payload.pull_request.number;
const owner = context.repo.owner;
const repo = context.repo.repo;
const baseUrl = `https://${owner}.github.io/${repo}`;
const previewPath = `/openapi/pr-${prNumber}/`;
const url = `${baseUrl}${previewPath}`;

const body = [
'📘 **OpenAPI / Swagger preview**',
'',
`➡️ ${url}`,
'',
'This page is automatically updated on each push to this PR.'
].join('\n');

await github.rest.issues.createComment({
owner,
repo,
issue_number: prNumber,
body,
});
Comment on lines +184 to +209

3 changes: 1 addition & 2 deletions app/Http/Utils/FilterAvailableSummitsStrategy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ final class FilterAvailableSummitsStrategy
*/
static public function shouldReturnAllSummits(IResourceServerContext $resource_server_ctx){
$scopes = $resource_server_ctx->getCurrentScope();
$current_realm = Config::get('app.scope_base_realm');
$needed_scope = sprintf(SummitScopes::ReadAllSummitData, $current_realm);
$needed_scope = SummitScopes::ReadAllSummitData;
return in_array($needed_scope, $scopes);
Comment on lines 28 to 31
}
}
3 changes: 1 addition & 2 deletions app/ModelSerializers/Summit/SummitSerializer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ public function serialize($expand = null, array $fields = [], array $relations =
// we had proper scopes
if (!$summit->isAvailableOnApi()) {
$scopes = $this->resource_server_context->getCurrentScope();
$current_realm = Config::get('app.url');
$needed_scope = sprintf(SummitScopes::ReadAllSummitData, $current_realm);
$needed_scope = SummitScopes::ReadAllSummitData;
if (!in_array($needed_scope, $scopes))
throw new HTTP403ForbiddenException;
}
Expand Down
4 changes: 4 additions & 0 deletions app/Providers/AppServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,10 @@ public function boot()
public function register()
{

if (!defined('SCOPE_BASE_REALM')) {
define('SCOPE_BASE_REALM', config('app.scope_base_realm'));
}
Comment on lines +721 to +723
Copy link

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Normalize and validate SCOPE_BASE_REALM before defining it.

Line 722 uses the raw config value directly. If it is empty or ends with /, generated scopes can become invalid/mismatched (e.g., /members/read or ...//members/read), which can break auth checks.

🔧 Proposed fix 
         if (!defined('SCOPE_BASE_REALM')) {
-            define('SCOPE_BASE_REALM', config('app.scope_base_realm'));
+            $scopeBaseRealm = rtrim((string) config('app.scope_base_realm', ''), '/');
+            if ($scopeBaseRealm === '') {
+                throw new \RuntimeException('Missing required config: app.scope_base_realm');
+            }
+            define('SCOPE_BASE_REALM', $scopeBaseRealm);
         }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/Providers/AppServiceProvider.php` around lines 721 - 723, Normalize and
validate the config value before defining SCOPE_BASE_REALM: read
config('app.scope_base_realm'), trim whitespace and remove any trailing slashes,
then ensure the resulting string is non-empty (or fail loudly/log an error)
before calling define('SCOPE_BASE_REALM', ...). Update the code around the
SCOPE_BASE_REALM definition in AppServiceProvider (where you currently call
define with the raw config value) to perform the trim/rtrim and non-empty check
first so generated scopes won't contain empty segments or trailing slashes.


App::singleton(IResourceServerContext::class, ResourceServerContext::class);

App::bind('resource_server_context', function ($app) {
Expand Down
4 changes: 2 additions & 2 deletions app/Security/CompanyScopes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
*/
final class CompanyScopes
{
const Read = '%s/companies/read';
const Write = '%s/companies/write';
const Read = SCOPE_BASE_REALM.'/companies/read';
const Write = SCOPE_BASE_REALM.'/companies/write';
}
2 changes: 1 addition & 1 deletion app/Security/GroupsScopes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@

class GroupsScopes
{
const ReadData = '%s/groups/read';
const ReadData = SCOPE_BASE_REALM.'/groups/read';
}
8 changes: 4 additions & 4 deletions app/Security/MemberScopes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
*/
final class MemberScopes
{
const ReadMemberData = '%s/members/read';
const ReadMemberData = SCOPE_BASE_REALM.'/members/read';

const ReadMyMemberData = '%s/members/read/me';
const ReadMyMemberData = SCOPE_BASE_REALM.'/members/read/me';

const WriteMemberData = '%s/members/write';
const WriteMemberData = SCOPE_BASE_REALM.'/members/write';

const WriteMyMemberData = '%s/members/write/me';
const WriteMyMemberData = SCOPE_BASE_REALM.'/members/write/me';
}
4 changes: 2 additions & 2 deletions app/Security/OrganizationScopes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
*/
final class OrganizationScopes
{
const ReadOrganizationData = '%s/organizations/read';
const WriteOrganizationData = '%s/organizations/write';
const ReadOrganizationData = SCOPE_BASE_REALM.'/organizations/read';
const WriteOrganizationData = SCOPE_BASE_REALM.'/organizations/write';
}
4 changes: 2 additions & 2 deletions app/Security/SponsoredProjectScope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
*/
final class SponsoredProjectScope
{
const Read = '%s/sponsored-projects/read';
const Write = '%s/sponsored-projects/write';
const Read = SCOPE_BASE_REALM.'/sponsored-projects/read';
const Write = SCOPE_BASE_REALM.'/sponsored-projects/write';
}
Loading
Loading