refactor: unify media extraction and secure credential lifecycle - #10
Merged
Conversation
ztygod
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors NoteForge’s media layer by consolidating YouTube/Bilibili extraction behind a single noteforge.media boundary, while introducing a more secure, lease-based lifecycle for temporary media and cookies (with optional encrypted retention via keyring).
Changes:
- Introduces
MediaService+ platform adapters + isolated worker execution to unify metadata/formats/playlists/subtitles/audio/video without exposing yt-dlp options. - Adds lease-scoped
MediaAssetand a newCookieServicethat filters domains and supports AEAD-encrypted retention with a keyring-managed master key. - Removes the legacy
noteforge.collectormodule, migrates CLI/pipeline/tests to the new APIs, and updates docs/config and dependencies.
Reviewed changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds cryptography/keyring and related transitive deps to support encrypted credential retention. |
| tests/media/test_media.py | Updates media tests to cover adapters, cookie filtering, assets, service download behavior, and repository round-trip. |
| tests/document/test_markdown_renderer.py | Updates imports to use noteforge.document as the public renderer/writer API. |
| tests/collector/test_inspection.py | Migrates inspection tests to noteforge.media.source. |
| tests/collector/test_bilibili.py | Migrates Bilibili tests from collector to the new adapter API. |
| tests/cli/test_app.py | Updates CLI test imports to use noteforge.media.source. |
| src/noteforge/renderer/init.py | Removes legacy renderer package entrypoint (API moved under noteforge.document). |
| src/noteforge/media/ytdlp/errors.py | Adds clarification docstring and keeps download error translation centralized. |
| src/noteforge/media/ytdlp/client.py | Hardens yt-dlp defaults (no browser cookie reading by options), adds explicit cookiefile injection only via service/worker. |
| src/noteforge/media/worker.py | Adds process-isolated worker implementation (plus in-process variant for tests). |
| src/noteforge/media/transcriber.py | Removes old transcriber protocol module (moved to protocols.py). |
| src/noteforge/media/subtitle.py | Adds clarifying docstrings around parsing and asset path expectations. |
| src/noteforge/media/source.py | Adds local URL inspection/normalization for YouTube/Bilibili with Bilibili page handling. |
| src/noteforge/media/service.py | Adds the unified MediaService orchestration layer, asset lifecycle, cookie lease usage, and CLI helper functions. |
| src/noteforge/media/repository.py | Renames cache to MediaRepository and clarifies scope (only small non-sensitive normalized data). |
| src/noteforge/media/protocols.py | Centralizes AudioTranscriber and MediaWorker protocols. |
| src/noteforge/media/platforms/youtube.py | Adds YouTube adapter implementation (currently minimal backend options). |
| src/noteforge/media/platforms/bilibili.py | Adds Bilibili adapter supplying required headers for requests. |
| src/noteforge/media/platforms/base.py | Adds the platform adapter base mapping raw yt-dlp structures into stable domain models. |
| src/noteforge/media/platforms/init.py | Exposes platform adapter types via package exports. |
| src/noteforge/media/models.py | Expands domain models (formats, requests, assets/auth enums) and clarifies the “no yt-dlp options leak” contract. |
| src/noteforge/media/cookies/service.py | Implements cookie leasing, domain filtering, encrypted retention, keyring-backed master key, and cleanup. |
| src/noteforge/media/cookies/policy.py | Adds per-platform cookie domain allowlists. |
| src/noteforge/media/cookies/init.py | Exposes cookie service/lease/index types via package exports. |
| src/noteforge/media/config.py | Refactors extractor config to separate network proxy config from credential handling and adds runtime/vault/worker settings. |
| src/noteforge/media/assets.py | Adds TTL-scoped MediaAsset with explicit persistence via export_to(). |
| src/noteforge/media/init.py | Defines the new public media API surface (MediaService, models, cookie types, inspection types). |
| src/noteforge/document/writer.py | Updates writer module docstring and aligns it with the document package boundary. |
| src/noteforge/document/renderers/markdown.py | Fixes imports to avoid circularity and anchors renderer to document.models. |
| src/noteforge/document/renderers/init.py | Introduces renderers package export for MarkdownRenderer. |
| src/noteforge/document/init.py | Exposes MarkdownRenderer and write_markdown from the document package. |
| src/noteforge/core/pipeline.py | Migrates pipeline to noteforge.media and noteforge.document public APIs. |
| src/noteforge/collector/source/inspection.py | Removes legacy source inspection implementation (replaced by noteforge.media.source). |
| src/noteforge/collector/source/init.py | Removes legacy source module exports. |
| src/noteforge/collector/platforms/youtube.py | Removes legacy YouTube collector. |
| src/noteforge/collector/platforms/bilibili.py | Removes legacy Bilibili collector. |
| src/noteforge/collector/platforms/base.py | Removes legacy platform collector base implementation. |
| src/noteforge/collector/platforms/init.py | Removes legacy collector platform exports. |
| src/noteforge/collector/factory.py | Removes legacy collector factory and CLI-facing wrappers (replaced by media service helpers). |
| src/noteforge/collector/init.py | Removes legacy collector package exports. |
| src/noteforge/cli/commands/inspect.py | Migrates CLI inspect command to noteforge.media. |
| src/noteforge/cli/commands/generate.py | Migrates CLI generate command to noteforge.media. |
| src/noteforge/cli/commands/doctor.py | Migrates CLI doctor command to noteforge.media. |
| README.zh-CN.md | Updates Chinese docs to describe the unified media service, worker isolation, and secure cookie lifecycle. |
| README.md | Adds English docs for media service + credentials (needs internal consistency update). |
| pyproject.toml | Adds cryptography and keyring runtime dependencies. |
| config.example.yaml | Updates example config to use runtime/vault/worker settings and removes plaintext cookie file options. |
Suppressed comments (2)
src/noteforge/media/cookies/service.py:285
_new_lease_root()creates the per-task lease directory viatempfile.mkdtemp()but does not explicitly set the lease directory permission to0700. That weakens the stated security guarantee (lease dir0700, cookie file0600) on platforms/filesystems wheremkdtemp()may be affected by the process umask.
self.runtime_root.mkdir(parents=True, exist_ok=True, mode=0o700)
os.chmod(self.runtime_root, 0o700)
return Path(tempfile.mkdtemp(prefix="lease-", dir=self.runtime_root))
src/noteforge/media/cookies/service.py:341
_vault_key()assumes the keyring value is valid hex and callsbytes.fromhex(stored)without error handling. If the keyring entry is corrupted/non-hex (or from an older format), this will raiseValueErrorand leak an implementation detail instead of a clearCookieSecurityErrorpath.
stored = keyring.get_password(service, account)
if stored is None:
stored = secrets.token_hex(32)
keyring.set_password(service, account, stored)
return bytes.fromhex(stored)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ) | ||
| self.vault_root = vault_root or Path.home() / ".noteforge" / "credentials" | ||
| self.lease_ttl = lease_ttl # 仅用于回收异常退出后的残留租约。 | ||
| self._lock = threading.RLock() |
Comment on lines
+379
to
+381
| def _asset_root(self) -> Path: | ||
| self.config.runtime_path.mkdir(parents=True, exist_ok=True) | ||
| return Path(tempfile.mkdtemp(prefix="asset-", dir=self.config.runtime_path)) |
Comment on lines
51
to
+55
| NoteForge downloads subtitles only. It does not download the video or audio. | ||
|
|
||
| ## Media service and credentials | ||
|
|
||
| `noteforge.media.MediaService` is the sole media boundary for YouTube and |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更概述
重构媒体获取架构，将 YouTube 和 Bilibili 的元数据、格式、播放列表、字幕、音频及视频能力统一收口到
noteforge.media。主要变更
MediaService公共 API，屏蔽 yt-dlp 参数和平台实现细节noteforge.collector模块，CLI 与核心流水线全部迁移到新媒体服务MediaAsset，通过 TTL 和上下文管理控制临时媒体生命周期export_to()显式持久化CookieService，负责 Cookie 获取、过滤、更新、保留和删除MediaRepositoryAudioTranscriber和MediaWorker接口集中到protocols.py安全设计
cookiesfrombrowser0600，租约目录权限为0700验证
git diff --check通过205 passed