From db5a431c4e5c8beec54ec93b866950575ac492c7 Mon Sep 17 00:00:00 2001 From: htilly Date: Tue, 14 Jul 2026 14:15:01 +0200 Subject: [PATCH] security: harden CI workflows, auth, setup surface, and deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address findings from a security review of the repo. Critical β€” GitHub Actions script injection (RCE): - Delete redundant enhance_feature_requests.yml, whose unguarded check-label job interpolated github.event.issue.body into a run: shell on every opened issue (any user -> arbitrary code on the runner). The remaining feature-request-enhance.yml is a functional superset. - feature-request-enhance.yml: pass all untrusted issue content via env vars (never spliced into run:), use random heredoc delimiters for $GITHUB_OUTPUT, and add least-privilege per-job permissions. - confluence-changelog.yml: move release body/fields to env and write with printf instead of a quoted heredoc. - generate-implementation.mjs: validate ISSUE_NUMBER (^[0-9]+$) before it is embedded in execSync/patch file paths. High β€” dependencies: - Bump undici override to ^7.28.0 and add form-data ^4.0.6 (both High advisories). High β€” setup surface: - isLocalRequest: treat any forwarded request as non-local so a same-host reverse proxy no longer makes every remote client look local and bypass the setup bootstrap token. - saveConfig: allowlist known keys; drop adminPasswordHash, __proto__, and any other injected/unknown fields. Medium: - auth-handler getClientIp: only trust X-Forwarded-For / X-Real-IP when the new trustProxy config is enabled (default off), so a forged header can no longer bypass login rate limiting. Falls back to the socket peer address. - webauthn-handler: the unauthenticated /status endpoint no longer returns credential-file contents (only exists/size/credentialCount); a configured webauthnOrigin is no longer overwritten by request headers. - setup validate-sonos: reject loopback, link-local (incl. 169.254.169.254 metadata), and multicast/reserved target IPs. Config/tests: - Add trustProxy to config.json.example and the admin allowlist (boolean). - Update auth-handler tests that asserted the old (insecure) X-Forwarded-For default to assert the secure-by-default behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/agent/generate-implementation.mjs | 6 +- .github/workflows/confluence-changelog.yml | 39 +- .../workflows/enhance_feature_requests.yml | 371 ------------------ .github/workflows/feature-request-enhance.yml | 117 ++++-- config/config.json.example | 2 + lib/admin-api.js | 3 +- lib/auth-handler.js | 30 +- lib/http-utils.js | 9 + lib/setup-handler.js | 61 ++- lib/webauthn-handler.js | 28 +- package-lock.json | 24 +- package.json | 3 +- test/auth-handler.test.mjs | 27 +- 13 files changed, 254 insertions(+), 466 deletions(-) delete mode 100644 .github/workflows/enhance_feature_requests.yml diff --git a/.github/agent/generate-implementation.mjs b/.github/agent/generate-implementation.mjs index 98a681b..eaa2652 100644 --- a/.github/agent/generate-implementation.mjs +++ b/.github/agent/generate-implementation.mjs @@ -16,7 +16,11 @@ const __dirname = dirname(fileURLToPath(import.meta.url)); */ const enhancedTask = process.env.ENHANCED_TASK || process.env.TASK || ""; -const issueNumber = process.env.ISSUE_NUMBER || "unknown"; +// issueNumber is embedded in file paths passed to execSync/patch below. Restrict it +// to digits so a workflow_dispatch caller cannot inject shell metacharacters or path +// traversal via ISSUE_NUMBER. Non-numeric values fall back to a safe constant. +const rawIssueNumber = process.env.ISSUE_NUMBER || ""; +const issueNumber = /^[0-9]+$/.test(rawIssueNumber) ? rawIssueNumber : "unknown"; const anthropicApiKey = process.env.ANTHROPIC_API_KEY; const model = process.env.CLAUDE_MODEL || "claude-sonnet-4-5-20250929"; diff --git a/.github/workflows/confluence-changelog.yml b/.github/workflows/confluence-changelog.yml index a3c5b38..3257a4f 100644 --- a/.github/workflows/confluence-changelog.yml +++ b/.github/workflows/confluence-changelog.yml @@ -30,29 +30,42 @@ jobs: - name: Get release info id: release + env: + EVENT_NAME: ${{ github.event_name }} + INPUT_VERSION: ${{ github.event.inputs.version }} + INPUT_DRY_RUN: ${{ github.event.inputs.dry_run }} + RELEASE_TAG: ${{ github.event.release.tag_name }} + RELEASE_NAME: ${{ github.event.release.name }} + RELEASE_URL: ${{ github.event.release.html_url }} + # Untrusted release body kept in an env var; never spliced into the shell. + RELEASE_BODY: ${{ github.event.release.body }} run: | - if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + if [ "$EVENT_NAME" = "workflow_dispatch" ]; then # Manual trigger - use input values - echo "version=${{ github.event.inputs.version }}" >> $GITHUB_OUTPUT - echo "name=SlackONOS ${{ github.event.inputs.version }}" >> $GITHUB_OUTPUT - echo "url=https://github.com/htilly/SlackONOS/releases/tag/${{ github.event.inputs.version }}" >> $GITHUB_OUTPUT - echo "dry_run=${{ github.event.inputs.dry_run }}" >> $GITHUB_OUTPUT + { + echo "version=$INPUT_VERSION" + echo "name=SlackONOS $INPUT_VERSION" + echo "url=https://github.com/htilly/SlackONOS/releases/tag/$INPUT_VERSION" + echo "dry_run=$INPUT_DRY_RUN" + } >> "$GITHUB_OUTPUT" echo "(Manual test run)" > release_body.txt else # Release trigger - echo "version=${{ github.event.release.tag_name }}" >> $GITHUB_OUTPUT - echo "name=${{ github.event.release.name }}" >> $GITHUB_OUTPUT - echo "url=${{ github.event.release.html_url }}" >> $GITHUB_OUTPUT - echo "dry_run=false" >> $GITHUB_OUTPUT - cat << 'EOF' > release_body.txt - ${{ github.event.release.body }} - EOF + { + echo "version=$RELEASE_TAG" + echo "name=$RELEASE_NAME" + echo "url=$RELEASE_URL" + echo "dry_run=false" + } >> "$GITHUB_OUTPUT" + printf '%s' "$RELEASE_BODY" > release_body.txt fi - name: Extract changelog for this version id: changelog + env: + RELEASE_TAG: ${{ github.event.release.tag_name }} run: | - VERSION="${{ github.event.release.tag_name }}" + VERSION="$RELEASE_TAG" # Remove 'v' prefix if present VERSION_NUM="${VERSION#v}" diff --git a/.github/workflows/enhance_feature_requests.yml b/.github/workflows/enhance_feature_requests.yml deleted file mode 100644 index 0d22d60..0000000 --- a/.github/workflows/enhance_feature_requests.yml +++ /dev/null @@ -1,371 +0,0 @@ -name: Enhance Feature Requests - -on: - issues: - types: [opened, labeled] - -jobs: - check-label: - runs-on: ubuntu-latest - outputs: - has_enhancement: ${{ steps.check.outputs.has_enhancement }} - already_enhanced: ${{ steps.check-enhanced.outputs.already_enhanced }} - steps: - - name: Check if issue has enhancement label - id: check - run: | - # For 'labeled' events, check if the label being added is 'enhancement' - # For 'opened' events, check if issue has 'enhancement' label - if [ "${{ github.event.action }}" = "labeled" ]; then - if [ "${{ github.event.label.name }}" = "enhancement" ]; then - echo "has_enhancement=true" >> $GITHUB_OUTPUT - else - echo "has_enhancement=false" >> $GITHUB_OUTPUT - fi - else - # For 'opened' events, check all labels - LABELS="${{ join(github.event.issue.labels.*.name, ',') }}" - if echo "$LABELS" | grep -q "enhancement"; then - echo "has_enhancement=true" >> $GITHUB_OUTPUT - else - echo "has_enhancement=false" >> $GITHUB_OUTPUT - fi - fi - - - name: Check if issue already enhanced - id: check-enhanced - run: | - # Check if issue already has enhanced content - ISSUE_BODY="${{ github.event.issue.body }}" - if echo "$ISSUE_BODY" | grep -q "Enhanced Feature Request"; then - echo "already_enhanced=true" >> $GITHUB_OUTPUT - echo "Issue already has enhanced content, skipping" - else - echo "already_enhanced=false" >> $GITHUB_OUTPUT - fi - - preprocess: - runs-on: ubuntu-latest - needs: check-label - if: needs.check-label.outputs.has_enhancement == 'true' && needs.check-label.outputs.already_enhanced == 'false' - permissions: - contents: read - outputs: - enhanced_task: ${{ steps.preprocess.outputs.enhanced_task }} - task_json: ${{ steps.preprocess.outputs.task_json }} - confluence_url: ${{ steps.preprocess.outputs.confluence_url }} - steps: - - name: Checkout master - uses: actions/checkout@v6 - with: - ref: master - - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version: 22.x - cache: 'npm' - cache-dependency-path: .github/agent/package-lock.json - - - name: Install agent dependencies - working-directory: .github/agent - run: npm install - - - name: Preprocess feature request to user story (OpenAI only) - id: preprocess - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - PREPROCESSING_MODEL: ${{ secrets.PREPROCESSING_MODEL }} - TASK: ${{ github.event.issue.title }} - REQUESTER: ${{ github.event.issue.user.login }} - # Skip Confluence creation in preprocessing - will be done in parallel job - CONFLUENCE_URL: "" - CONFLUENCE_EMAIL: "" - CONFLUENCE_API_TOKEN: "" - CONFLUENCE_SPACE_KEY: "" - CONFLUENCE_PARENT_PAGE_ID: "" - GITHUB_RUN_ID: ${{ github.run_id }} - run: | - OUTPUT=$(node .github/agent/preprocess-task.mjs 2>&1) - echo "$OUTPUT" - - # Extract enhanced task from output - if echo "$OUTPUT" | grep -q "ENHANCED_TASK_START"; then - ENHANCED_TASK=$(echo "$OUTPUT" | sed -n '/ENHANCED_TASK_START/,/ENHANCED_TASK_END/p' | sed '1d;$d') - echo "enhanced_task<> $GITHUB_OUTPUT - echo "$ENHANCED_TASK" >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT - else - # Fallback to original task - echo "enhanced_task=${{ github.event.issue.title }}" >> $GITHUB_OUTPUT - fi - - # Extract JSON if available - JSON_LINE=$(echo "$OUTPUT" | grep "ENHANCED_TASK_JSON:" || true) - if [ -n "$JSON_LINE" ]; then - JSON_DATA=$(echo "$JSON_LINE" | sed 's/ENHANCED_TASK_JSON://') - echo "task_json=$JSON_DATA" >> $GITHUB_OUTPUT - else - echo "task_json=null" >> $GITHUB_OUTPUT - fi - - create-confluence: - runs-on: ubuntu-latest - needs: preprocess - if: needs.preprocess.outcome == 'success' - permissions: - contents: read - steps: - - name: Checkout master - uses: actions/checkout@v6 - with: - ref: master - - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version: 22.x - cache: 'npm' - cache-dependency-path: .github/agent/package-lock.json - - - name: Install agent dependencies - working-directory: .github/agent - run: npm install - - - name: Create Confluence page - id: confluence - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - PREPROCESSING_MODEL: ${{ secrets.PREPROCESSING_MODEL }} - TASK: ${{ github.event.issue.title }} - REQUESTER: ${{ github.event.issue.user.login }} - CONFLUENCE_URL: ${{ secrets.CONFLUENCE_URL }} - CONFLUENCE_EMAIL: ${{ secrets.CONFLUENCE_EMAIL }} - CONFLUENCE_API_TOKEN: ${{ secrets.CONFLUENCE_API_TOKEN }} - CONFLUENCE_SPACE_KEY: ${{ secrets.CONFLUENCE_SPACE_KEY }} - CONFLUENCE_PARENT_PAGE_ID: ${{ secrets.CONFLUENCE_PARENT_PAGE_ID }} - GITHUB_RUN_ID: ${{ github.run_id }} - # Use enhanced task from preprocessing - ENHANCED_TASK: ${{ needs.preprocess.outputs.enhanced_task }} - run: | - # Create a temporary script that only creates Confluence page - cat > /tmp/create-confluence.mjs << 'SCRIPT_EOF' - // Use native fetch (Node 20+) - - const enhancedTask = process.env.ENHANCED_TASK || ""; - const task = process.env.TASK || ""; - const requester = process.env.REQUESTER || "unknown"; - const confluenceUrl = process.env.CONFLUENCE_URL || ""; - const confluenceEmail = process.env.CONFLUENCE_EMAIL || ""; - const confluenceApiToken = process.env.CONFLUENCE_API_TOKEN || ""; - const confluenceSpaceKey = process.env.CONFLUENCE_SPACE_KEY || "AICODE"; - const confluenceParentPageId = process.env.CONFLUENCE_PARENT_PAGE_ID || ""; - const githubRunId = process.env.GITHUB_RUN_ID || "unknown"; - - if (!confluenceUrl || !confluenceEmail || !confluenceApiToken) { - console.log("[CONFLUENCE] Confluence credentials not configured, skipping page creation"); - process.exit(0); - } - - try { - const timestamp = new Date().toISOString().split('T')[0]; - const pageTitle = `AICODE: ${task.substring(0, 80)} (${timestamp})`; - - const storageContent = ` -

Original Task

-

${task.replace(/&/g, '&').replace(//g, '>').replace(/\n/g, '
')}

- -

User Story

-

${enhancedTask.replace(/&/g, '&').replace(//g, '>').replace(/\n/g, '
')}

- -

Implementation Status

-

YellowIn Progress

- -

Related Links

-

GitHub Run: View Workflow

- `.trim(); - - const auth = Buffer.from(`${confluenceEmail}:${confluenceApiToken}`).toString('base64'); - - const pageData = { - type: 'page', - title: pageTitle, - space: { key: confluenceSpaceKey }, - body: { - storage: { - value: storageContent, - representation: 'storage' - } - } - }; - - if (confluenceParentPageId) { - pageData.ancestors = [{ id: confluenceParentPageId }]; - } - - const response = await fetch(`${confluenceUrl}/rest/api/content`, { - method: 'POST', - headers: { - 'Authorization': `Basic ${auth}`, - 'Content-Type': 'application/json', - 'Accept': 'application/json' - }, - body: JSON.stringify(pageData) - }); - - if (!response.ok) { - const errorText = await response.text(); - console.error(`[CONFLUENCE] Failed to create page: ${response.status} ${response.statusText}`); - console.error(`[CONFLUENCE] Error details: ${errorText}`); - process.exit(1); - } - - const data = await response.json(); - const pageUrl = `${confluenceUrl}/wiki${data._links.webui}`; - console.log(`[CONFLUENCE] βœ… Confluence page created: ${pageUrl}`); - console.log(`CONFLUENCE_URL:${pageUrl}`); - } catch (error) { - console.error(`[CONFLUENCE] Error creating page: ${error.message}`); - process.exit(1); - } - SCRIPT_EOF - - OUTPUT=$(node /tmp/create-confluence.mjs 2>&1) - echo "$OUTPUT" - - # Extract Confluence URL - CONFLUENCE_URL=$(echo "$OUTPUT" | grep "CONFLUENCE_URL:" | sed 's/CONFLUENCE_URL://' || echo "N/A") - echo "confluence_url=$CONFLUENCE_URL" >> $GITHUB_OUTPUT - - update-github-issue: - runs-on: ubuntu-latest - needs: preprocess - if: needs.preprocess.outcome == 'success' - permissions: - contents: read - issues: write - steps: - - name: Update GitHub issue with enhanced description - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - ISSUE_NUMBER: ${{ github.event.issue.number }} - run: | - # Read enhanced task from JSON output to avoid shell quoting issues - TASK_JSON="${{ needs.preprocess.outputs.task_json }}" - - if [ -n "$TASK_JSON" ] && [ "$TASK_JSON" != "null" ]; then - # Extract values from JSON using jq - ENHANCED_TASK=$(echo "$TASK_JSON" | jq -r '.enhanced // empty') - ORIGINAL_TASK=$(echo "$TASK_JSON" | jq -r '.original // empty') - else - # Fallback to outputs if JSON not available - ENHANCED_TASK="${{ needs.preprocess.outputs.enhanced_task }}" - ORIGINAL_TASK="${{ github.event.issue.title }}" - fi - - # Get original body (may be null/empty) - ORIGINAL_BODY="${{ github.event.issue.body }}" - if [ -z "$ORIGINAL_BODY" ] || [ "$ORIGINAL_BODY" = "null" ]; then - ORIGINAL_BODY="_(No description provided)_" - fi - - # Build enhanced body with proper escaping using jq - # Start with base content (without Confluence link for now) - BASE_BODY=$(jq -nr \ - --arg enhanced "$ENHANCED_TASK" \ - --arg original "$ORIGINAL_BODY" \ - --arg requester "${{ github.event.issue.user.login }}" \ - --arg created "${{ github.event.issue.created_at }}" \ - '"## πŸ“ Enhanced Feature Request\n\n" + $enhanced + "\n\n---\n\n## πŸ“‹ Original Request\n\n" + $original + "\n\n---\n\n**Requested by:** " + $requester + " \n**Created:** " + $created') - - ENHANCED_BODY="$BASE_BODY" - - # Use jq to properly construct JSON payload for PATCH - PAYLOAD=$(jq -n \ - --arg body "$ENHANCED_BODY" \ - '{body: $body}') - - # Update the issue - curl -X PATCH "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}" \ - -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ - -H "Accept: application/vnd.github+json" \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" - - echo "βœ… Issue #${{ github.event.issue.number }} updated with enhanced description" - - add-confluence-link: - runs-on: ubuntu-latest - needs: [preprocess, create-confluence, update-github-issue] - if: needs.create-confluence.outcome == 'success' && needs.update-github-issue.outcome == 'success' - permissions: - issues: write - steps: - - name: Add Confluence link to issue - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - CONFLUENCE_URL="${{ needs.create-confluence.outputs.confluence_url }}" - - if [ "$CONFLUENCE_URL" = "N/A" ] || [ -z "$CONFLUENCE_URL" ]; then - echo "No Confluence URL to add" - exit 0 - fi - - # Get current issue body - CURRENT_BODY=$(curl -s "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}" \ - -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ - -H "Accept: application/vnd.github+json" | jq -r '.body') - - # Add Confluence link to the end - UPDATED_BODY=$(jq -nr \ - --arg current "$CURRENT_BODY" \ - --arg confluence "$CONFLUENCE_URL" \ - '$current + "\n\nπŸ“„ **Requirements documented:** " + $confluence') - - # Update issue with Confluence link - PAYLOAD=$(jq -n \ - --arg body "$UPDATED_BODY" \ - '{body: $body}') - - curl -X PATCH "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}" \ - -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ - -H "Accept: application/vnd.github+json" \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" - - echo "βœ… Added Confluence link to issue" - - add-comment: - runs-on: ubuntu-latest - needs: [preprocess, create-confluence, update-github-issue] - if: always() && (needs.preprocess.outcome == 'success' || needs.update-github-issue.outcome == 'success') - permissions: - issues: write - steps: - - name: Add comment to issue - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - CONFLUENCE_URL="${{ needs.create-confluence.outputs.confluence_url }}" - - # Build comment with proper escaping using jq - if [ "$CONFLUENCE_URL" != "N/A" ] && [ -n "$CONFLUENCE_URL" ]; then - COMMENT=$(jq -nr \ - --arg confluence "$CONFLUENCE_URL" \ - '"πŸ€– **Feature request enhanced!**\n\nThis feature request has been automatically enhanced with a structured user story format.\n\nπŸ“„ Requirements documented: " + $confluence + "\n\nThe issue description has been updated with acceptance criteria and technical notes."') - else - COMMENT=$(jq -nr \ - '"πŸ€– **Feature request enhanced!**\n\nThis feature request has been automatically enhanced with a structured user story format.\n\nThe issue description has been updated with acceptance criteria and technical notes."') - fi - - # Use jq to properly construct JSON payload - PAYLOAD=$(jq -n \ - --arg body "$COMMENT" \ - '{body: $body}') - - curl -X POST "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/comments" \ - -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ - -H "Accept: application/vnd.github+json" \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" diff --git a/.github/workflows/feature-request-enhance.yml b/.github/workflows/feature-request-enhance.yml index 0eba59c..54cb2c1 100644 --- a/.github/workflows/feature-request-enhance.yml +++ b/.github/workflows/feature-request-enhance.yml @@ -16,14 +16,16 @@ concurrency: group: enhance-feature-request-${{ github.event.issue.number || inputs.issue_number }} cancel-in-progress: true +# Least-privilege default. Individual jobs elevate only what they need. permissions: - issues: write - contents: write - pull-requests: write + contents: read jobs: check-label: runs-on: ubuntu-latest + permissions: + contents: read + issues: read outputs: has_enhancement: ${{ steps.check.outputs.has_enhancement }} already_enhanced: ${{ steps.check.outputs.already_enhanced }} @@ -32,11 +34,19 @@ jobs: steps: - name: Set issue number id: set_issue + env: + EVENT_NAME: ${{ github.event_name }} + DISPATCH_ISSUE_NUMBER: ${{ inputs.issue_number }} + EVENT_ISSUE_NUMBER: ${{ github.event.issue.number }} run: | - if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - echo "issue_number=${{ inputs.issue_number }}" >> "$GITHUB_OUTPUT" + if [ "$EVENT_NAME" = "workflow_dispatch" ]; then + # workflow_dispatch requires repo write access; still validate it is numeric. + case "$DISPATCH_ISSUE_NUMBER" in + ''|*[!0-9]*) echo "Invalid issue number: $DISPATCH_ISSUE_NUMBER" >&2; exit 1 ;; + esac + echo "issue_number=$DISPATCH_ISSUE_NUMBER" >> "$GITHUB_OUTPUT" else - echo "issue_number=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT" + echo "issue_number=$EVENT_ISSUE_NUMBER" >> "$GITHUB_OUTPUT" fi - name: Check if opened event should be skipped @@ -44,17 +54,20 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ github.event.issue.number }} + EVENT_NAME: ${{ github.event_name }} + EVENT_ACTION: ${{ github.event.action }} + REPO: ${{ github.repository }} run: | # Default to not skipping (for labeled events and workflow_dispatch) SKIP_VALUE="false" - + # If issue is opened with enhancement label already present, # skip this run (labeled event will handle it to avoid duplicate runs) - if [ "${{ github.event_name }}" = "issues" ] && [ "${{ github.event.action }}" = "opened" ]; then + if [ "$EVENT_NAME" = "issues" ] && [ "$EVENT_ACTION" = "opened" ]; then ISSUE_JSON=$(curl -s \ -H "Authorization: token $GITHUB_TOKEN" \ -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER") + "https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER") LABELS=$(echo "$ISSUE_JSON" | jq -r '.labels[].name' | tr '\n' ' ') if echo "$LABELS" | grep -qw "enhancement"; then echo "⚠️ Issue opened with enhancement label already present - skipping (labeled event will handle it)" @@ -68,6 +81,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ steps.set_issue.outputs.issue_number }} + REPO: ${{ github.repository }} run: | set -e @@ -75,7 +89,7 @@ jobs: ISSUE_JSON=$(curl -s \ -H "Authorization: token $GITHUB_TOKEN" \ -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER") + "https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER") # Write to temp file for safe parsing echo "$ISSUE_JSON" > /tmp/issue.json @@ -106,6 +120,9 @@ jobs: runs-on: ubuntu-latest needs: check-label if: needs.check-label.outputs.should_skip != 'true' && needs.check-label.outputs.has_enhancement == 'true' && needs.check-label.outputs.already_enhanced == 'false' + permissions: + contents: read + issues: read outputs: enhanced_task: ${{ steps.preprocess.outputs.enhanced_task }} issue_title: ${{ steps.fetch.outputs.issue_title }} @@ -132,11 +149,12 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + REPO: ${{ github.repository }} run: | ISSUE_JSON=$(curl -s \ -H "Authorization: token $GITHUB_TOKEN" \ -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER") + "https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER") TITLE=$(echo "$ISSUE_JSON" | jq -r '.title') USER=$(echo "$ISSUE_JSON" | jq -r '.user.login') @@ -146,10 +164,12 @@ jobs: echo "issue_title=$TITLE" >> "$GITHUB_OUTPUT" echo "issue_user=$USER" >> "$GITHUB_OUTPUT" echo "issue_created=$CREATED" >> "$GITHUB_OUTPUT" + # Random heredoc delimiter so a body line "EOF" cannot inject step outputs. + DELIM="EOF_$(openssl rand -hex 16)" { - echo "issue_body<> "$GITHUB_OUTPUT" - name: Preprocess issue @@ -171,15 +191,18 @@ jobs: ENHANCED="$TASK" fi + DELIM="EOF_$(openssl rand -hex 16)" { - echo "enhanced_task<> "$GITHUB_OUTPUT" create-confluence: runs-on: ubuntu-latest needs: [check-label, preprocess] + permissions: + contents: read outputs: confluence_url: ${{ steps.confluence.outputs.confluence_url }} steps: @@ -220,21 +243,32 @@ jobs: update-issue: runs-on: ubuntu-latest needs: [check-label, preprocess, create-confluence] + permissions: + issues: write steps: - name: Update issue body env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + REPO: ${{ github.repository }} + # Untrusted issue-derived content is passed via env vars and never + # interpolated into the shell, preventing script injection. + ENHANCED_TASK: ${{ needs.preprocess.outputs.enhanced_task }} + CONFLUENCE_URL: ${{ needs.create-confluence.outputs.confluence_url }} + ORIGINAL_BODY: ${{ needs.preprocess.outputs.issue_body }} + REQUESTER: ${{ needs.preprocess.outputs.issue_user }} + CREATED: ${{ needs.preprocess.outputs.issue_created }} run: | set -euo pipefail - # Use jq to safely build JSON with proper escaping + # Use jq to safely build JSON with proper escaping. All values are read + # from environment variables ("$VAR"), never spliced into the run: block. jq -n \ - --arg enhanced "${{ needs.preprocess.outputs.enhanced_task }}" \ - --arg confluence "${{ needs.create-confluence.outputs.confluence_url }}" \ - --arg original "${{ needs.preprocess.outputs.issue_body }}" \ - --arg requester "${{ needs.preprocess.outputs.issue_user }}" \ - --arg created "${{ needs.preprocess.outputs.issue_created }}" \ + --arg enhanced "$ENHANCED_TASK" \ + --arg confluence "$CONFLUENCE_URL" \ + --arg original "$ORIGINAL_BODY" \ + --arg requester "$REQUESTER" \ + --arg created "$CREATED" \ '{ body: ( "## πŸ“ Enhanced Feature Request\n\n" + @@ -255,20 +289,21 @@ jobs: -H "Accept: application/vnd.github+json" \ -H "Content-Type: application/json" \ -d @- \ - "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER" + "https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER" echo "βœ… Issue updated with enhanced + original content" - - add-comment: runs-on: ubuntu-latest needs: [check-label, update-issue] + permissions: + issues: write steps: - name: Comment env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + REPO: ${{ github.repository }} run: | COMMENT="πŸ€– **Feature request enhanced!** @@ -280,7 +315,7 @@ jobs: -H "Accept: application/vnd.github+json" \ -H "Content-Type: application/json" \ -d @- \ - "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER/comments" + "https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER/comments" generate-implementation: runs-on: ubuntu-latest @@ -317,6 +352,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ISSUE_NUMBER: ${{ needs.check-label.outputs.issue_number }} + REPO: ${{ github.repository }} PREPROCESS_OUTCOME: ${{ needs.preprocess.outcome }} PREPROCESS_ENHANCED_TASK: ${{ needs.preprocess.outputs.enhanced_task }} PREPROCESS_ISSUE_TITLE: ${{ needs.preprocess.outputs.issue_title }} @@ -324,13 +360,14 @@ jobs: # If preprocess succeeded, use its enriched output if [ "$PREPROCESS_OUTCOME" = "success" ] && [ -n "$PREPROCESS_ENHANCED_TASK" ]; then echo "βœ… Using enriched task from preprocess" + DELIM="EOF_$(openssl rand -hex 16)" { - echo "enhanced_task<> "$GITHUB_OUTPUT" else # Preprocess skipped/failed, fetch issue data as fallback @@ -338,16 +375,17 @@ jobs: ISSUE_JSON=$(curl -s \ -H "Authorization: token $GITHUB_TOKEN" \ -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${{ github.repository }}/issues/$ISSUE_NUMBER") + "https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER") TITLE=$(echo "$ISSUE_JSON" | jq -r '.title') BODY=$(echo "$ISSUE_JSON" | jq -r '.body // ""') + DELIM="EOF_$(openssl rand -hex 16)" { - echo "enhanced_task<> "$GITHUB_OUTPUT" fi @@ -398,11 +436,11 @@ jobs: # Code was applied - add all changed files echo "βœ… Code changes were applied, staging all changes..." git add -A - + # Get list of changed files for commit message CHANGED=$(git diff --cached --name-only | tr '\n' ',' | sed 's/,$//') echo "Changed files: $CHANGED" - + # Create commit message COMMIT_MSG=$(cat <=224). RFC1918 private and normal LAN/public + * addresses are allowed. + */ +function isSafeSonosTarget(value) { + const [a, b] = value.split('.').map(Number); + if (a === 0) return false; + if (a === 127) return false; + if (a === 169 && b === 254) return false; + if (a >= 224) return false; + return true; +} + function hasSlackConfig(config) { return typeof config.slackAppToken === 'string' && config.slackAppToken.startsWith('xapp-') && @@ -265,8 +305,25 @@ async function saveConfig(configData) { existingConfig = {}; } - // Merge with provided config (configData) on top of existing + defaults - const finalConfig = { ...defaultConfig, ...existingConfig, ...(configData || {}) }; + // Drop any keys not on the setup allowlist before merging, so a save-config + // payload can only touch known configuration (never adminPasswordHash, + // prototype-pollution keys, or arbitrary injected fields). + const providedConfig = (configData && typeof configData === 'object') ? configData : {}; + const sanitizedConfig = {}; + const rejectedKeys = []; + for (const key of Object.keys(providedConfig)) { + if (SETUP_ALLOWED_KEYS.has(key)) { + sanitizedConfig[key] = providedConfig[key]; + } else { + rejectedKeys.push(key); + } + } + if (rejectedKeys.length > 0 && global.logger && typeof global.logger.warn === 'function') { + global.logger.warn(`Setup save-config ignored disallowed keys: ${rejectedKeys.join(', ')}`); + } + + // Merge sanitized config on top of existing + defaults + const finalConfig = { ...defaultConfig, ...existingConfig, ...sanitizedConfig }; // Ensure config directory exists const configDir = path.dirname(CONFIG_PATH); diff --git a/lib/webauthn-handler.js b/lib/webauthn-handler.js index 14ed8b1..0930ba3 100644 --- a/lib/webauthn-handler.js +++ b/lib/webauthn-handler.js @@ -68,6 +68,10 @@ function getRPConfig(req) { rpId = hostname; } + // If an origin is explicitly configured (webauthnOrigin), honor it and do not + // let request headers (Host/X-Forwarded-Host) override the phishing-binding origin. + const originConfigured = Boolean(origin); + let portSegment = ''; if (hostPort) { const portNum = Number(hostPort); @@ -89,7 +93,9 @@ function getRPConfig(req) { } } - origin = `${protocol}://${hostname}${portSegment}`; + if (!originConfigured) { + origin = `${protocol}://${hostname}${portSegment}`; + } } if (!origin) { @@ -714,30 +720,32 @@ async function verifyAuthenticationResponseHandler(req, body) { async function getCredentialsFileInfo() { try { const fileExists = await fs.access(WEBAUTHN_CREDENTIALS_PATH, fsSync.constants.F_OK).then(() => true).catch(() => false); - let content = null; let fileSize = 0; - + let credentialCount = 0; + if (fileExists) { try { const fileContent = await fs.readFile(WEBAUTHN_CREDENTIALS_PATH, 'utf8'); fileSize = fileContent.length; - content = JSON.parse(fileContent); + const parsed = JSON.parse(fileContent); + credentialCount = Array.isArray(parsed.credentials) ? parsed.credentials.length : 0; } catch (err) { - content = { error: 'Failed to read/parse file: ' + err.message }; + // Ignore read/parse errors; never leak file contents to callers. } } - + + // NOTE: /api/auth/webauthn/status is reachable WITHOUT authentication, so this + // exposes only counts/metadata β€” never the credential records (public keys, + // device names, timestamps) or the absolute file path. return { - path: WEBAUTHN_CREDENTIALS_PATH, exists: fileExists, size: fileSize, - content: content + credentialCount }; } catch (err) { return { - path: WEBAUTHN_CREDENTIALS_PATH, exists: false, - error: err.message + credentialCount: 0 }; } } diff --git a/package-lock.json b/package-lock.json index 7bdb02b..353db82 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,7 @@ "@slack/web-api": "^7.16.0", "bcrypt": "^6.0.0", "discord.js": "^14.26.4", - "lodash": "^4.18.1", + "lodash": "^4.17.21", "mp3-duration": "^1.1.0", "nconf": "^0.13.0", "openai": "^6.42.0", @@ -1507,16 +1507,16 @@ } }, "node_modules/form-data": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.5.tgz", - "integrity": "sha512-8RipRLol37bNs2bhoV67fiTEvdTrbMUYcFTiy3+wuuOnUog2QBHCZWXDRijWQfAkhBj2Uf5UnVaiWwA5vdd82w==", + "version": "4.0.6", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.6.tgz", + "integrity": "sha512-vKatAh4SlVfgbv+YtmhiRjhEMJsYpsG1Y2rMQtR+SVSbytsSD1YGzDIcrAJmdFec88u/+VoGmxnl+80gL1tRCQ==", "license": "MIT", "dependencies": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", "es-set-tostringtag": "^2.1.0", - "hasown": "^2.0.2", - "mime-types": "^2.1.12" + "hasown": "^2.0.4", + "mime-types": "^2.1.35" }, "engines": { "node": ">= 6" @@ -1649,9 +1649,9 @@ } }, "node_modules/hasown": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.2.tgz", - "integrity": "sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ==", + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.4.tgz", + "integrity": "sha512-T2UbfbBEF32wiepXIsMlTW9+dDYC6wMh/t/vYA4tuOMKqWz/n3vr1NFSxQiyP+zk2mXsoMA/i/7qV6LKut1t1A==", "license": "MIT", "dependencies": { "function-bind": "^1.1.2" @@ -3048,9 +3048,9 @@ } }, "node_modules/undici": { - "version": "7.27.1", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.27.1.tgz", - "integrity": "sha512-UDdpiex+mzigiyrXrGbiUaF4HzTNhKbh2vRNFaTMzcqmLIPrZxaCtwo/1TMSuWoM1Xz3WiTo9KdgI3kRqYzJGg==", + "version": "7.28.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.28.0.tgz", + "integrity": "sha512-cRZYrTDwWznlnRiPjggAGxZXanty6M8RV1ff8Wm4LWXBp7/IG8v5DnOm74DtUBp9OONpK75YlPnIjQqX0dBDtA==", "license": "MIT", "engines": { "node": ">=20.18.1" diff --git a/package.json b/package.json index b0663d6..9f79227 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,8 @@ "overrides": { "axios": "^1.17.0", "serialize-javascript": "^7.0.5", - "undici": "^7.0.0", + "undici": "^7.28.0", + "form-data": "^4.0.6", "diff": "^8.0.3", "ip": "^2.0.1" } diff --git a/test/auth-handler.test.mjs b/test/auth-handler.test.mjs index a589797..e59a6a5 100644 --- a/test/auth-handler.test.mjs +++ b/test/auth-handler.test.mjs @@ -335,24 +335,26 @@ describe('Auth Handler', function() { }); describe('Client IP Detection', function() { - it('should extract IP from x-forwarded-for header', function() { + it('ignores x-forwarded-for by default (trustProxy disabled) and uses the socket peer', function() { const req = { headers: { 'x-forwarded-for': '203.0.113.195, 70.41.3.18, 150.172.238.178' }, connection: { remoteAddress: '127.0.0.1' } }; - + + // Forwarded headers are client-controlled; trusting them by default would + // let an attacker forge a unique IP per request and bypass rate limiting. const ip = authHandler.getClientIp(req); - expect(ip).to.equal('203.0.113.195'); + expect(ip).to.equal('127.0.0.1'); }); - - it('should extract IP from x-real-ip header', function() { + + it('ignores x-real-ip by default (trustProxy disabled) and uses the socket peer', function() { const req = { headers: { 'x-real-ip': '203.0.113.195' }, connection: { remoteAddress: '127.0.0.1' } }; - + const ip = authHandler.getClientIp(req); - expect(ip).to.equal('203.0.113.195'); + expect(ip).to.equal('127.0.0.1'); }); it('should fall back to connection.remoteAddress', function() { @@ -382,14 +384,15 @@ describe('Auth Handler', function() { expect(ip).to.equal('unknown'); }); - it('should trim whitespace from x-forwarded-for', function() { + it('does not fall back to forwarded headers when no socket address is present', function() { const req = { - headers: { 'x-forwarded-for': ' 203.0.113.195 , 70.41.3.18' }, - connection: { remoteAddress: '127.0.0.1' } + headers: { 'x-forwarded-for': ' 203.0.113.195 , 70.41.3.18' } }; - + + // With no trusted proxy and no socket/connection address, the spoofable + // forwarded header must not be used; result is 'unknown'. const ip = authHandler.getClientIp(req); - expect(ip).to.equal('203.0.113.195'); + expect(ip).to.equal('unknown'); }); });