From b550b73083e794154e21eef29d8d34777d752875 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 16 Sep 2026 09:51:21 -0700 Subject: [PATCH 1/2] fix(actions): support fork checkouts and PAT pushes in assign-rfc-number - Set `allow-unsafe-pr-checkout: true` on the **isolated** `target/` sparse checkout step to opt into `actions/checkout` v7's fork safety check. - Authenticate git operations via `gh auth setup-git` using `FLUTTERACTIONSBOT_RFC_TOKEN` (with fallback to `GITHUB_TOKEN`) so commits can be pushed to fork PR branches where maintainer edits are enabled. - Update PR labels (`rfc-assigned`) atomically before pushing changes so `rfc-lint` passes on the triggered `synchronize` event. - Add top-level `permissions: {}` to enforce least-privilege defaults. - Improve failure fallback comments with direct workflow run log links and local `dart run bin/assign_rfc_number.dart` instructions when pushing to a fork fails. --- .github/workflows/assign-rfc-number.yml | 97 ++++++++++++++++++++----- 1 file changed, 79 insertions(+), 18 deletions(-) diff --git a/.github/workflows/assign-rfc-number.yml b/.github/workflows/assign-rfc-number.yml index 5dfeaa8..4b545b5 100644 --- a/.github/workflows/assign-rfc-number.yml +++ b/.github/workflows/assign-rfc-number.yml @@ -11,14 +11,21 @@ on: pull_request_target: # zizmor: ignore[dangerous-triggers] Isolated two-checkout model prevents code execution from untrusted PR types: [labeled] +# Workflow-level default permissions: +# Explicitly set to empty ({}) to enforce least privilege by default. +# Any job added to this file starts with zero permissions unless explicitly +# granted in its own job-level `permissions:` block below. +permissions: {} + jobs: assign-number: + name: assign-number if: github.event.label.name == 'assign-rfc-number' runs-on: ubuntu-latest permissions: - contents: write - pull-requests: write - issues: write + contents: write # Required to push assigned RFC commit to same-repository PR branches + pull-requests: write # Required to add/remove RFC lifecycle labels on the PR + issues: write # Required to post status and fallback comments on the PR steps: # --- Sandbox 1: Trusted Tooling Setup --- @@ -47,13 +54,27 @@ jobs: sparse-checkout-cone-mode: false fetch-depth: 0 persist-credentials: false + # Note: `allow-unsafe-pr-checkout: true` opts out of actions/checkout v7's blanket + # block on fork checkouts in `pull_request_target`. This is safe because: + # 1. We only sparse-checkout `rfc/` markdown documents into a separate `target/` directory. + # 2. `persist-credentials: false` prevents token leakage into `.git/config`. + # 3. No code, scripts, or dependencies from `target/` are ever executed. + allow-unsafe-pr-checkout: true + # Configures the GitHub CLI credential helper (`gh auth setup-git`) using + # FLUTTERACTIONSBOT_RFC_TOKEN (falling back to GITHUB_TOKEN if unset) so + # remote git operations (`git fetch`, `git push`) authenticate without + # embedding credentials into `.git/config` or remote URLs. - name: Configure Git working-directory: target run: | - git config user.name "github-actions[bot]" - git config user.email "github-actions[bot]@users.noreply.github.com" - git fetch "https://github.com/${{ github.repository }}.git" main:origin/main + gh auth setup-git + git config --global user.name "flutteractionsbot" + git config --global user.email "" + git fetch "https://github.com/${BASE_REPO}.git" main:origin/main + env: + BASE_REPO: ${{ github.repository }} + GH_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_RFC_TOKEN || secrets.GITHUB_TOKEN }} # --- Execution & Delivery --- - name: Assign RFC Number @@ -61,37 +82,77 @@ jobs: working-directory: target run: dart run ../tools/bin/assign_rfc_number.dart + # Update labels BEFORE pushing changes: + # When pushing with FLUTTERACTIONSBOT_RFC_TOKEN (a user PAT), GitHub immediately + # triggers `pull_request` (`synchronize`) workflows like `rfc-lint.yml`. + # Applying `rfc-assigned` first ensures `rfc-lint` sees the label and permits + # the newly assigned non-0000 RFC number. + - name: Update PR Labels + if: success() + run: | + gh pr edit "$PR_NUMBER" --repo "$REPO" \ + --add-label rfc-assigned \ + --remove-label assign-rfc-number || true + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + - name: Commit and Push Changes if: success() working-directory: target + run: | + git add -A rfc/ + git commit -m "docs(rfc): assign RFC ${RFC_ID}" + git push "https://github.com/${REPO}.git" "HEAD:${PR_HEAD_REF}" env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_RFC_TOKEN || secrets.GITHUB_TOKEN }} RFC_ID: ${{ steps.assign.outputs.rfc_id }} PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} REPO: ${{ github.event.pull_request.head.repo.full_name }} - run: | - git add -A rfc/ - git commit -m "docs(rfc): assign RFC ${RFC_ID}" - git push "https://x-access-token:${GH_TOKEN}@github.com/${REPO}.git" "HEAD:${PR_HEAD_REF}" - name: Handle Success if: success() + run: | + gh pr comment "$PR_NUMBER" --repo "$REPO" --body "Assigned RFC ${RFC_ID}." || true env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} RFC_ID: ${{ steps.assign.outputs.rfc_id }} PR_NUMBER: ${{ github.event.pull_request.number }} REPO: ${{ github.repository }} - run: | - gh pr edit "$PR_NUMBER" --repo "$REPO" --remove-label assign-rfc-number || true - gh pr edit "$PR_NUMBER" --repo "$REPO" --add-label rfc-assigned || true - gh pr comment "$PR_NUMBER" --repo "$REPO" --body "Assigned RFC ${RFC_ID}." || true - name: Handle Failure if: failure() + run: | + gh pr edit "$PR_NUMBER" --repo "$REPO" --remove-label assign-rfc-number || true + if [ -n "$RFC_ID" ]; then + cat < comment.txt + Assigned **RFC ${RFC_ID}**, but failed to automatically push the commit to your pull request branch (this commonly happens if "Allow edits from maintainers" is disabled on a fork PR, or if the fork belongs to an organization). + + Please examine the [workflow run logs](${RUN_URL}) and run the following commands locally on your branch to apply the assigned RFC number: + + \`\`\`sh + dart run bin/assign_rfc_number.dart + git add -A rfc/ + git commit -m "docs(rfc): assign RFC ${RFC_ID}" + git push + \`\`\` + EOF + else + cat < comment.txt + Failed to automatically assign an RFC number. + + Please examine the [workflow run logs](${RUN_URL}) to review the error details, or run the tool locally to diagnose and assign your RFC number: + + \`\`\`sh + dart run bin/assign_rfc_number.dart + \`\`\` + EOF + fi + gh pr comment "$PR_NUMBER" --repo "$REPO" --body-file comment.txt || true env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.pull_request.number }} REPO: ${{ github.repository }} - run: | - gh pr edit "$PR_NUMBER" --repo "$REPO" --remove-label assign-rfc-number || true - gh pr comment "$PR_NUMBER" --repo "$REPO" --body "Failed to automatically assign RFC number. Check workflow logs for details." || true + RFC_ID: ${{ steps.assign.outputs.rfc_id }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} From 8e1bb974df17171fe7594b67fda75c3a77a8b8c5 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 16 Sep 2026 17:07:40 -0700 Subject: [PATCH 2/2] second pass security notes 1. only execute tools from tools folder (working folder considered safe) 2. update assigner to take target directory 3. reduce global permissions in each file 4. read files in as bash array and use possix `-- ` 5. percent encode messages to github --- .github/workflows/assign-rfc-number.yml | 44 ++++----- .github/workflows/rfc-lint.yml | 24 ++--- .github/workflows/test.yml | 2 + .github/workflows/validate-rfc-number.yml | 2 + bin/assign_rfc_number.dart | 8 +- lib/src/assigner.dart | 72 ++++++++++++--- lib/src/github_annotation.dart | 15 ++- lib/src/linter.dart | 21 ++++- lib/src/validator.dart | 11 ++- test/assign_rfc_number_test.dart | 107 +++++++++++++++++++++- test/github_annotation_test.dart | 16 ++++ 11 files changed, 267 insertions(+), 55 deletions(-) diff --git a/.github/workflows/assign-rfc-number.yml b/.github/workflows/assign-rfc-number.yml index 4b545b5..8a5b754 100644 --- a/.github/workflows/assign-rfc-number.yml +++ b/.github/workflows/assign-rfc-number.yml @@ -5,7 +5,8 @@ name: RFC Number Assigner # workflow implements a strict two-checkout isolation model using separate sibling directories: # 1. tools/: Contains trusted tooling from main. dart pub get runs only here. # 2. target/: Contains only rfc/ markdown content from the PR branch via sparse checkout. -# 3. dart run ../tools/bin/assign_rfc_number.dart runs from target/ executing only trusted bytecode. +# 3. dart run bin/assign_rfc_number.dart runs from tools/ (passing --target-dir="../target/rfc") +# executing only trusted bytecode and package configurations. # 4. Gated by the maintainer-applied 'assign-rfc-number' label. on: pull_request_target: # zizmor: ignore[dangerous-triggers] Isolated two-checkout model prevents code execution from untrusted PR @@ -61,26 +62,14 @@ jobs: # 3. No code, scripts, or dependencies from `target/` are ever executed. allow-unsafe-pr-checkout: true - # Configures the GitHub CLI credential helper (`gh auth setup-git`) using - # FLUTTERACTIONSBOT_RFC_TOKEN (falling back to GITHUB_TOKEN if unset) so - # remote git operations (`git fetch`, `git push`) authenticate without - # embedding credentials into `.git/config` or remote URLs. - - name: Configure Git - working-directory: target - run: | - gh auth setup-git - git config --global user.name "flutteractionsbot" - git config --global user.email "" - git fetch "https://github.com/${BASE_REPO}.git" main:origin/main - env: - BASE_REPO: ${{ github.repository }} - GH_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_RFC_TOKEN || secrets.GITHUB_TOKEN }} - # --- Execution & Delivery --- + # Runs strictly in the trusted 'tools' directory so Dart resolves package + # configs from 'tools/' and git ls-tree queries 'origin/main' directly from + # the trusted repository checkout before any git credentials are configured. - name: Assign RFC Number id: assign - working-directory: target - run: dart run ../tools/bin/assign_rfc_number.dart + working-directory: tools + run: dart run bin/assign_rfc_number.dart --target-dir="../target/rfc" # Update labels BEFORE pushing changes: # When pushing with FLUTTERACTIONSBOT_RFC_TOKEN (a user PAT), GitHub immediately @@ -98,13 +87,26 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} REPO: ${{ github.repository }} + # Configures the GitHub CLI credential helper (`gh auth setup-git`) using + # FLUTTERACTIONSBOT_RFC_TOKEN (falling back to GITHUB_TOKEN if unset) so + # remote git operations (`git push`) authenticate without embedding + # credentials into `.git/config` or remote URLs. + - name: Configure Git + if: success() + run: | + gh auth setup-git + git config --global user.name "flutteractionsbot" + git config --global user.email "" + env: + GH_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_RFC_TOKEN || secrets.GITHUB_TOKEN }} + - name: Commit and Push Changes if: success() working-directory: target run: | - git add -A rfc/ - git commit -m "docs(rfc): assign RFC ${RFC_ID}" - git push "https://github.com/${REPO}.git" "HEAD:${PR_HEAD_REF}" + git -c core.hooksPath=/dev/null add -A rfc/ + git -c core.hooksPath=/dev/null commit --no-verify -m "docs(rfc): assign RFC ${RFC_ID}" + git -c core.hooksPath=/dev/null push "https://github.com/${REPO}.git" "HEAD:${PR_HEAD_REF}" env: GH_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_RFC_TOKEN || secrets.GITHUB_TOKEN }} RFC_ID: ${{ steps.assign.outputs.rfc_id }} diff --git a/.github/workflows/rfc-lint.yml b/.github/workflows/rfc-lint.yml index e4faf37..52a79a9 100644 --- a/.github/workflows/rfc-lint.yml +++ b/.github/workflows/rfc-lint.yml @@ -11,13 +11,14 @@ on: paths: - 'rfc/**' +permissions: {} + jobs: lint-rfcs: name: lint-rfcs runs-on: ubuntu-latest permissions: contents: read - pull-requests: read steps: - name: Checkout Code @@ -35,8 +36,6 @@ jobs: - name: Get Changed RFCs id: changed-rfcs if: github.event_name == 'pull_request' - env: - BASE_REF: ${{ github.base_ref }} run: | BASE_TARGET="origin/${BASE_REF:-main}" if ! git rev-parse --verify "$BASE_TARGET" >/dev/null 2>&1; then @@ -47,33 +46,34 @@ jobs: FILES=$(git diff --name-only --diff-filter=ACMR "$BASE_TARGET" -- 'rfc/*.md' 2>/dev/null || true) fi if [ -n "$FILES" ]; then + EOF_MARKER="EOF_$(openssl rand -hex 16)" echo "has_changes=true" >> "$GITHUB_OUTPUT" { - echo "files<> "$GITHUB_OUTPUT" else echo "has_changes=false" >> "$GITHUB_OUTPUT" fi + env: + BASE_REF: ${{ github.base_ref }} - name: Run RFC Linter (PR Mode) if: github.event_name == 'pull_request' && steps.changed-rfcs.outputs.has_changes == 'true' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} - CHANGED_FILES: ${{ steps.changed-rfcs.outputs.files }} run: | + mapfile -t FILES_ARRAY <<< "$CHANGED_FILES" dart run bin/rfc_lint.dart \ --enforce-drafts \ --labels "$LABELS" \ --github-actions \ - $CHANGED_FILES + -- "${FILES_ARRAY[@]}" + env: + LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} + CHANGED_FILES: ${{ steps.changed-rfcs.outputs.files }} - name: Run RFC Linter (Merge Queue & Main Mode) if: github.event_name != 'pull_request' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | dart run bin/rfc_lint.dart \ --github-actions diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2195988..a11d129 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,6 +16,8 @@ on: - 'analysis_options.yaml' - '.github/workflows/test.yml' +permissions: {} + jobs: test: name: Dart Format, Analyze, and Test diff --git a/.github/workflows/validate-rfc-number.yml b/.github/workflows/validate-rfc-number.yml index b372970..1f5720c 100644 --- a/.github/workflows/validate-rfc-number.yml +++ b/.github/workflows/validate-rfc-number.yml @@ -11,6 +11,8 @@ on: paths: - 'rfc/**' +permissions: {} + jobs: validate-rfc-number: name: validate-rfc-number diff --git a/bin/assign_rfc_number.dart b/bin/assign_rfc_number.dart index 413c800..42a26a7 100644 --- a/bin/assign_rfc_number.dart +++ b/bin/assign_rfc_number.dart @@ -9,6 +9,11 @@ import 'package:rfc_tools/src/assigner.dart'; void main(List arguments) async { final parser = ArgParser() + ..addOption( + 'target-dir', + defaultsTo: RfcAssigner.rfcDir, + help: 'Directory containing RFC markdown documents.', + ) ..addOption( 'target-file', help: @@ -42,11 +47,12 @@ void main(List arguments) async { return; } + final targetDir = results.option('target-dir') ?? RfcAssigner.rfcDir; final targetFile = results.rest.firstOrNull ?? results.option('target-file'); final dryRun = results.flag('dry-run'); const fs = LocalFileSystem(); - final assigner = RfcAssigner(fs: fs); + final assigner = RfcAssigner(fs: fs, rfcDirPath: targetDir); try { stdout.writeln('Assigning RFC number...'); diff --git a/lib/src/assigner.dart b/lib/src/assigner.dart index db52cb0..7283586 100644 --- a/lib/src/assigner.dart +++ b/lib/src/assigner.dart @@ -43,10 +43,12 @@ class RfcAssigner { final FileSystem fs; final GitListFunction gitList; + final String rfcDirPath; const RfcAssigner({ required this.fs, this.gitList = RfcAssigner.defaultGitList, + this.rfcDirPath = RfcAssigner.rfcDir, }); /// Discovers RFC filenames in main branch via git. @@ -100,16 +102,17 @@ class RfcAssigner { Future> getMainFiles() async => cachedMainFiles ??= await _getMainBranchRfcFiles(); - final dir = fs.directory(rfcDir); + final rfcDir = fs.directory(rfcDirPath); + await _assertSafeDirectory(rfcDir); // Identify the target RFC file to assign. final RfcFile targetRfc; List? cachedEntries; if (targetPath != null) { - targetRfc = await _resolveExplicitTarget(targetPath, dir); + targetRfc = await _resolveExplicitTarget(targetPath, rfcDir); } else { - (targetRfc, cachedEntries) = await _discoverTarget(dir, getMainFiles); + (targetRfc, cachedEntries) = await _discoverTarget(rfcDir, getMainFiles); } final category = targetRfc.category!; @@ -121,8 +124,14 @@ class RfcAssigner { final prefix = '$category.'; final targetBasename = p.basename(targetRfc.path); - final entries = cachedEntries ?? await dir.list().toList(); + final entries = + cachedEntries ?? await rfcDir.list(followLinks: false).toList(); for (final entry in entries) { + if (entry is Link || await fs.isLink(entry.path)) { + throw StateError( + 'Symbolic links are not permitted in the RFC directory: "${entry.path}".', + ); + } if (entry is! File) continue; final fileName = p.basename(entry.path); if (fileName == targetBasename) continue; @@ -172,6 +181,9 @@ class RfcAssigner { final newFileName = '$category.$newIndexStr-${targetRfc.slug}.md'; final newPath = p.join(p.dirname(targetRfc.path), newFileName); + await _assertSafeFile(targetRfc.path, rfcDir); + await _assertSafeFile(newPath, rfcDir); + // Execute filesystem changes if not dryRun if (!dryRun) { final oldFile = fs.file(targetRfc.path); @@ -196,10 +208,40 @@ class RfcAssigner { ); } + Future _assertSafeDirectory(Directory dir) async { + if (await fs.isLink(dir.path)) { + throw StateError( + 'RFC directory "${dir.path}" must not be a symbolic link.', + ); + } + if (!await dir.exists()) { + throw StateError('RFC directory "$rfcDirPath" does not exist.'); + } + } + + Future _assertSafeFile(String path, Directory baseDir) async { + if (await fs.isLink(path)) { + throw StateError('RFC file "$path" must not be a symbolic link.'); + } + final canonicalBase = await baseDir.resolveSymbolicLinks(); + final parentDir = fs.directory(p.dirname(path)); + if (await fs.isLink(parentDir.path)) { + throw StateError( + 'Parent directory of "$path" must not be a symbolic link.', + ); + } + final canonicalParent = await parentDir.resolveSymbolicLinks(); + if (!p.equals(canonicalBase, canonicalParent) && + !p.isWithin(canonicalBase, canonicalParent)) { + throw StateError('RFC file "$path" resolves outside of "$rfcDirPath".'); + } + } + Future _resolveExplicitTarget( String targetPath, Directory dir, ) async { + await _assertSafeFile(targetPath, dir); final targetFile = fs.file(targetPath); if (!await targetFile.exists()) { throw ArgumentError('Target RFC file "$targetPath" does not exist.'); @@ -211,9 +253,6 @@ class RfcAssigner { '"AAA.NNNN-.md".', ); } - if (!await dir.exists()) { - throw StateError('RFC directory "$rfcDir" does not exist.'); - } final content = await targetFile.readAsString(); return RfcFile.parse(content, path: targetPath); } @@ -222,12 +261,14 @@ class RfcAssigner { Directory dir, Future> Function() getMainFiles, ) async { - if (!await dir.exists()) { - throw StateError('RFC directory "$rfcDir" does not exist.'); - } - final cachedEntries = await dir.list().toList(); + final cachedEntries = await dir.list(followLinks: false).toList(); final draftFiles = []; for (final entry in cachedEntries) { + if (entry is Link || await fs.isLink(entry.path)) { + throw StateError( + 'Symbolic links are not permitted in the RFC directory: "${entry.path}".', + ); + } if (entry is File) { final fileName = p.basename(entry.path); final match = RfcFile.filenamePattern.firstMatch(fileName); @@ -239,11 +280,12 @@ class RfcAssigner { if (draftFiles.length == 1) { final draftFile = draftFiles.first; + await _assertSafeFile(draftFile.path, dir); final content = await draftFile.readAsString(); return (RfcFile.parse(content, path: draftFile.path), cachedEntries); } else if (draftFiles.length > 1) { throw StateError( - 'Multiple draft RFCs (.0000) found in "$rfcDir". ' + 'Multiple draft RFCs (.0000) found in "$rfcDirPath". ' 'Specify --target-file explicitly.', ); } else { @@ -251,7 +293,7 @@ class RfcAssigner { // RFC that collides with main (e.g. another PR merged with the same number while // this PR was in review). The validator prevents the collision from landing in main; // this auto-discovers the colliding file so [assign] can reallocate it. - final targetRfc = await _reallocate(cachedEntries, getMainFiles); + final targetRfc = await _reallocate(dir, cachedEntries, getMainFiles); return (targetRfc, cachedEntries); } } @@ -270,6 +312,7 @@ class RfcAssigner { /// colliding RFC so [assign] can re-allocate it to the next available number /// (e.g. `110.0043`) without requiring the author to manually revert to `.0000`. Future _reallocate( + Directory dir, List cachedEntries, Future> Function() getMainFiles, ) async { @@ -307,6 +350,7 @@ class RfcAssigner { if (collidingFiles.length == 1) { final collidingFile = collidingFiles.first; + await _assertSafeFile(collidingFile.path, dir); final content = await collidingFile.readAsString(); return RfcFile.parse(content, path: collidingFile.path); } else if (collidingFiles.length > 1) { @@ -315,7 +359,7 @@ class RfcAssigner { ); } else { throw StateError( - 'No RFC requiring number assignment found in "$rfcDir". ' + 'No RFC requiring number assignment found in "$rfcDirPath". ' 'Draft RFCs must use index ".0000" to be assigned a number.', ); } diff --git a/lib/src/github_annotation.dart b/lib/src/github_annotation.dart index 96df73c..922309a 100644 --- a/lib/src/github_annotation.dart +++ b/lib/src/github_annotation.dart @@ -19,6 +19,16 @@ extension GithubAnnotationExtension on String { ).replaceAll('\r', '%0D').replaceAll('\n', '%0A'); } + /// Encodes special characters (`%`, `\r`, `\n`, `:`, `,`) in this string per + /// GitHub Actions workflow command property specifications (`escapeProperty`). + String toGithubWorkflowProperty() { + return replaceAll('%', '%25') + .replaceAll('\r', '%0D') + .replaceAll('\n', '%0A') + .replaceAll(':', '%3A') + .replaceAll(',', '%2C'); + } + /// Formats this string message as a GitHub Actions workflow annotation. /// /// Example: @@ -41,11 +51,12 @@ extension GithubAnnotationExtension on String { String? title, }) { final encoded = toGithubWorkflowValue(); + final escapedPath = filePath.toGithubWorkflowProperty(); final params = [ - 'file=$filePath', + 'file=$escapedPath', if (line != null) 'line=$line', if (column != null) 'col=$column', - if (title != null) 'title=$title', + if (title != null) 'title=${title.toGithubWorkflowProperty()}', ].join(','); return '::$type $params::$encoded'; diff --git a/lib/src/linter.dart b/lib/src/linter.dart index ebdab04..de1921e 100644 --- a/lib/src/linter.dart +++ b/lib/src/linter.dart @@ -61,6 +61,16 @@ class RfcLinter { final issues = []; final relativePath = file.path; + if (await fs.isLink(file.path)) { + issues.add( + LintIssue( + filePath: relativePath, + message: 'Symbolic links are not permitted in the RFC directory.', + ), + ); + return issues; + } + if (!await file.exists()) { issues.add(LintIssue(filePath: relativePath, message: 'File not found.')); return issues; @@ -205,11 +215,18 @@ class RfcLinter { return issues; } - final entries = await dir.list().toList(); + final entries = await dir.list(followLinks: false).toList(); entries.sort((a, b) => a.path.compareTo(b.path)); for (final entry in entries) { - if (entry is File && entry.path.endsWith('.md')) { + if (entry is Link || await fs.isLink(entry.path)) { + issues.add( + LintIssue( + filePath: entry.path, + message: 'Symbolic links are not permitted in the RFC directory.', + ), + ); + } else if (entry is File && entry.path.endsWith('.md')) { issues.addAll(await lintFile(entry)); } } diff --git a/lib/src/validator.dart b/lib/src/validator.dart index 9611d02..0176167 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -108,13 +108,20 @@ class RfcValidator { return (isSuccess: false, errors: errors); } - final entries = await dir.list().toList(); + final entries = await dir.list(followLinks: false).toList(); entries.sort((a, b) => a.path.compareTo(b.path)); // Validate file-level naming/draft invariants and group valid RFCs by category (AAA). final rfcsByCategory = >{}; for (final entry in entries) { - if (entry is File && entry.path.endsWith('.md')) { + if (entry is Link || await fs.isLink(entry.path)) { + errors.add( + ValidationError( + filePath: entry.path, + message: 'Symbolic links are not permitted in the RFC directory.', + ), + ); + } else if (entry is File && entry.path.endsWith('.md')) { final rfc = RfcFile.fromPath(entry.path); _validateFileStructure( diff --git a/test/assign_rfc_number_test.dart b/test/assign_rfc_number_test.dart index 970569d..598ef2e 100644 --- a/test/assign_rfc_number_test.dart +++ b/test/assign_rfc_number_test.dart @@ -462,9 +462,114 @@ authors: [https://github.com/octocat] ); }); - test('default constructor uses RfcAssigner.defaultGitList', () { + test('default constructor uses RfcAssigner.defaultGitList and rfcDir', () { final assigner = RfcAssigner(fs: fs); expect(assigner.gitList, equals(RfcAssigner.defaultGitList)); + expect(assigner.rfcDirPath, equals('rfc')); + }); + + test( + 'assigns RFC number using custom rfcDirPath (isolated directory execution)', + () async { + final targetDir = await fs + .directory('workspace/target/rfc') + .create(recursive: true); + await targetDir + .childFile('110.0000-isolated-feature.md') + .writeAsString(rfcBody('110.0000', 'Isolated Feature')); + + final simulatedMain = { + 'rfc/110.0001-existing-main.md', + 'rfc/110.0002-existing-main-2.md', + }; + + final assigner = RfcAssigner( + fs: fs, + rfcDirPath: 'workspace/target/rfc', + gitList: ({String baseBranch = 'origin/main'}) async => simulatedMain, + ); + + final result = await assigner.assign(); + + expect(result.category, equals('110')); + expect(result.newIndex, equals(3)); + expect( + result.newPath, + equals('workspace/target/rfc/110.0003-isolated-feature.md'), + ); + expect( + await fs + .file('workspace/target/rfc/110.0000-isolated-feature.md') + .exists(), + isFalse, + ); + expect( + await fs + .file('workspace/target/rfc/110.0003-isolated-feature.md') + .exists(), + isTrue, + ); + }, + ); + + test('throws StateError when custom rfcDirPath does not exist', () async { + final assigner = RfcAssigner( + fs: fs, + rfcDirPath: 'nonexistent/target/rfc', + ); + expect( + () => assigner.assign(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('nonexistent/target/rfc'), + ), + ), + ); + }); + + test('rejects symbolic links inside the RFC directory', () async { + await fs.file('secret.txt').writeAsString('SENSITIVE_RUNNER_DATA'); + await fs.link('rfc/110.0000-symlink-exploit.md').create('../secret.txt'); + + final assigner = createAssigner(); + expect( + () => assigner.assign(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Symbolic links are not permitted'), + ), + ), + ); + }); + + test('rejects pre-existing destination symlink overwrite attacks', () async { + await fs + .file('rfc/110.0000-exploit.md') + .writeAsString(rfcBody('110.0000', 'Exploit')); + await fs.file('secret.txt').writeAsString('SENSITIVE_RUNNER_DATA'); + // Attacker creates a symlink at the predicted target path 110.0001-exploit.md + await fs.link('rfc/110.0001-exploit.md').create('../secret.txt'); + + final assigner = createAssigner(); + expect( + () => assigner.assign(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Symbolic links are not permitted'), + ), + ), + ); + // Verify secret.txt was not overwritten + expect( + await fs.file('secret.txt').readAsString(), + equals('SENSITIVE_RUNNER_DATA'), + ); }); test( diff --git a/test/github_annotation_test.dart b/test/github_annotation_test.dart index 53eeb88..5bf9a9d 100644 --- a/test/github_annotation_test.dart +++ b/test/github_annotation_test.dart @@ -132,6 +132,22 @@ void main() { ), ); }); + + test( + 'percent-encodes colons and commas in filePath and title properties', + () { + final annotation = 'Property injection attempt'.toGithubAnnotation( + filePath: 'rfc/evil,title=Hacked::Injected.md', + title: 'Title: With, Special%Chars', + ); + expect( + annotation, + equals( + '::error file=rfc/evil%2Ctitle=Hacked%3A%3AInjected.md,title=Title%3A With%2C Special%25Chars::Property injection attempt', + ), + ); + }, + ); }); });