Harden discovery, OAuth, transcoding, and CI - #181
Conversation
|
@copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
🟡 Changes recommended
There are workflow and runtime edge-case bugs (CI image tagging/pushing, transcode env parsing to NaN disabling safeguards, and failed-source channel merging potentially duplicating state) that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens several reliability/security-critical surfaces of the IPTV Proxy (HDHomeRun discovery + source refresh behavior, OAuth client lifecycle, ffmpeg transcoding, and CI container scanning), and checks in design/review evidence artifacts to document the current state and follow-up work.
Changes:
- Add bounded source-metadata fetching + preserve cached HDHomeRun channels when refresh stalls; move startup refresh to background.
- Invalidate OAuth auth codes/tokens when clients are removed/disabled/lose scope; improve MCP bearer auth behavior when OAuth is disabled.
- Harden transcoding process management and update CI to scan the exact PR-built image; add SECURITY policy and planning/review artifacts.
File summaries
| File | Description |
|---|---|
| test/unit/hdhr.test.js | Adds unit coverage for discovery BaseURL/LineupURL selection (LAN Host, custom port, configured base_url, forwarded headers). |
| test/integration/parseM3U-sqlite.test.js | Adds regression coverage for stalled HDHomeRun discovery/lineup and validates cached-channel retention; cleans env/test hooks. |
| test/integration/oauth.test.js | Adds integration coverage ensuring removed/disabled/out-of-scope clients lose access without impacting other clients. |
| test-scoped.vue | Adds a standalone scoped-style Vue SFC (appears to be a misc test/fixture). |
| server/transcode.js | Adds active-worker cap, timeouts, stderr bounds, backpressure handling, and avoids Windows cmd.exe shell invocation. |
| server/oauth.js | Adjusts MCP bearer auth erroring when OAuth is disabled but a bearer token is presented. |
| server/hdhr.js | Implements request-derived and configurable BaseURL resolution for HDHomeRun discovery via shared getBaseUrl logic. |
| server/config.js | Switches HDHomeRun metadata fetches to the new bounded fetch policy helper. |
| SECURITY.md | Documents CI security posture (lint/format/audit blocking; Trivy scan policy; exception process). |
| scripts/parseM3U.js | Uses bounded fetch for HDHomeRun metadata; preserves cached channels on failed refreshes; returns richer per-source result. |
| scripts/check-channel-health.js | Disables ambient HTTP proxying for LAN/tuner health probes. |
| review/2026-09-06/review-tests.log | Captures test-run output as review evidence. |
| review/2026-09-06/review-lint.log | Captures lint output as review evidence. |
| review/2026-09-06/review-integration.log | Captures integration-run output as review evidence. |
| review/2026-09-06/reproduce.mjs | Adds isolated reproduction probes used during review. |
| review/2026-09-06/README.md | Summarizes review scope, findings, and issue backlog for follow-up work. |
| review/2026-09-06/issues.json | Machine-readable list of created issues from the review. |
| review/2026-09-06/issue-1.md | Review issue write-up artifact. |
| review/2026-09-06/issue-2.md | Review issue write-up artifact. |
| review/2026-09-06/issue-3.md | Review issue write-up artifact. |
| review/2026-09-06/issue-4.md | Review issue write-up artifact. |
| review/2026-09-06/issue-5.md | Review issue write-up artifact. |
| review/2026-09-06/issue-6.md | Review issue write-up artifact. |
| review/2026-09-06/issue-7.md | Review issue write-up artifact. |
| review/2026-09-06/issue-8.md | Review issue write-up artifact. |
| review/2026-09-06/issue-9.md | Review issue write-up artifact. |
| review/2026-09-06/issue-10.md | Review issue write-up artifact. |
| review/2026-09-06/issue-11.md | Review issue write-up artifact. |
| review/2026-09-06/issue-12.md | Review issue write-up artifact. |
| review/2026-09-06/issue-13.md | Review issue write-up artifact. |
| review/2026-09-06/issue-14.md | Review issue write-up artifact. |
| review/2026-09-06/issue-15.md | Review issue write-up artifact. |
| review/2026-09-06/issue-16.md | Review issue write-up artifact. |
| review/2026-09-06/issue-17.md | Review issue write-up artifact. |
| review/2026-09-06/created-issues.jsonl | Records created issue URLs from the review session. |
| public/admin/index.html | Normalizes minor formatting in the admin entrypoint. |
| plan/web-app-design-plan.md | Adds a long-form design plan describing target architecture and gaps. |
| libs/source-fetch-policy.js | Introduces a shared bounded axios GET helper using timeout + AbortController. |
| libs/oauth-service.js | Adds enabled support, tightens token/code validation against current client config/scope. |
| libs/config-loader.js | Extends app schema to allow OAuth client enabled flag. |
| index.js | Moves source discovery to background so startup isn’t blocked by stalled tuners. |
| config/examples/app.example.yaml | Documents enabled/scope behavior and token invalidation expectations. |
| admin/package.json | Bumps cindor UI dependencies. |
| admin/package-lock.json | Locks cindor UI dependency bumps. |
| .github/workflows/ci.yml | Removes continue-on-error from checks; loads PR image locally and scans it via Trivy with enforcement. |
Review details
Files not reviewed (1)
- admin/package-lock.json: Generated file
Suppressed comments (2)
.github/workflows/ci.yml:166
- After removing the local-only
iptv-proxy-ci:*tag, the scan steps should reference a tag that is guaranteed to exist locally whenload: true. Since metadata-action already generates atype=shatag, you can scan the deterministic fully-qualifiedghcr.io/${{ github.repository }}:${{ github.sha }}tag instead of depending on an extra tag.
SCAN_IMAGE: iptv-proxy-ci:${{ github.sha }}
BUILD_DIGEST: ${{ steps.build.outputs.digest }}
.github/workflows/ci.yml:182
- Trivy currently scans
iptv-proxy-ci:${{ github.sha }}, which only exists because the build step added that extra local tag. If you drop that local tag (to avoid pushing it on non-PR events), update Trivy to scan the fully-qualified sha tag that buildx loads locally (the same one metadata-action generates).
with:
image-ref: iptv-proxy-ci:${{ github.sha }}
format: 'sarif'
output: 'trivy-results.sarif'
- Files reviewed: 40/46 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const MAX_ACTIVE_WORKERS = Number.parseInt(process.env.TRANSCODE_MAX_WORKERS || '3', 10); | ||
| const STARTUP_TIMEOUT_MS = Number.parseInt(process.env.TRANSCODE_STARTUP_TIMEOUT_MS || '15000', 10); | ||
| const IDLE_TIMEOUT_MS = Number.parseInt(process.env.TRANSCODE_IDLE_TIMEOUT_MS || '30000', 10); |
| tags: | | ||
| ${{ steps.meta.outputs.tags }} | ||
| iptv-proxy-ci:${{ github.sha }} |
| const failedSourceNames = new Set( | ||
| sourceResults.filter(result => !result.succeeded).map(result => result.sourceName) | ||
| ); | ||
| const cachedFailedSourceChannels = loadChannelSnapshot().filter(channel => | ||
| failedSourceNames.has(channel.source) | ||
| ); | ||
| const allChannels = [ | ||
| ...sourceResults.flatMap(result => result.channels), | ||
| ...cachedFailedSourceChannels, | ||
| ]; |
| <template> | ||
| <div class="wrapper"> | ||
| <cindor-button>Test</cindor-button> | ||
| </div> |
# Conflicts: # admin/package-lock.json # admin/package.json Co-authored-by: cbulock <176519+cbulock@users.noreply.github.com>
Resolved by merging |
Summary
Validation
The admin build was intentionally not run.