feat(harness): 외부 하네스 팩 받기(local/git) + 갈아치우기 + trust 게이트#46
Conversation
하네스 소스를 단일 고정 레포에서 "교체 가능한 팩"으로. 블록 소스/유닛 디렉터리 해상도를 pack_dir() 한 곳으로 모아 active_pack(config)에 따라 builtin(pharos repo/claude) 또는 레지스트리 팩(~/.config/pharos/harnesses/<id>) 을 가리키게 함. 컴포넌트 구조는 불변(회귀 0) — 소스만 갈아끼움. - 매니페스트: pharos-harness.json(name/version/blocks/units) 디스크립터. builtin()이 레거시 하드코딩 컴포넌트를 그대로 재현(parity 테스트로 가드). - 커맨드: harness_list_packs(빌트인+레지스트리), set_active_pack(검증→전환→재적용). - UI: Harness 페이지 상단 Active Pack 스위처, 전환 시 매니저 리로드. - 테스트: 매니페스트 parity/roundtrip, 팩 스왑(소스 버전 교체)·복귀·불완전팩 거부. env 변경 테스트 직렬화 Mutex 추가. 다음: 외부 fetch(git URL/로컬) + trust 게이트(L1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
외부 하네스 "받기"의 첫 절반 — 로컬 폴더를 팩으로 추가하고 신뢰 모델로
활성화를 게이트.
- 레지스트리: ~/.config/pharos/harnesses/registry.json (팩 dir 바깥 →
악성 팩이 자기 trust 위조 불가). PackMeta{id, source, trusted}.
- add_pack_from_path: 검증 → 레지스트리에 self-contained 스냅샷 복사 →
trusted 기록(사용자 자기 파일). remove_pack(활성 시 builtin 복귀).
- trust 게이트: builtin 외 팩은 trusted여야 활성화 — hooks가 제3자 코드라.
set_pack_trusted로 토글.
- PackInfo에 trusted/source 추가. UI: 팩 추가 입력 + 신뢰/해제/제거 버튼,
미신뢰 팩 활성화 차단.
- 테스트: add/activate/untrust-block/remove(17 passed). env 테스트 직렬화.
다음: git URL clone(L1b) + L2 폴리시.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
"받기"의 나머지 절반 — git clone으로 원격 하네스를 받는다. - install_pack 공통화(local/git 공유). add_pack_from_git: git clone --depth 1 → 클론 안에서 팩 루트 탐색(루트 또는 claude/ 서브디렉터리) → 레지스트리에 복사 → UNTRUSTED 기록(원격 hooks = 제3자 코드, 신뢰 전 활성화 차단). - find_pack_root: pharos 레이아웃(claude/) 자동 인식. - UI: git URL 입력 + clone 버튼(미신뢰로 추가 안내). - 테스트: 로컬 git remote clone → untrusted → 활성화 차단(18 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 1 minute and 32 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a harness pack management system end-to-end. ChangesHarness Pack Management System
Sequence Diagram(s)sequenceDiagram
participant User
participant HarnessPacks
rect rgba(100, 149, 237, 0.5)
Note over User,HarnessPacks: Add a pack from git
User->>HarnessPacks: Enter git URL, click Add
HarnessPacks->>harness_backend: add_harness_pack_from_git(url)
harness_backend->>TempDir: git clone url
harness_backend->>Registry: validate & copy pack, save registry.json
harness_backend-->>HarnessPacks: Vec<PackInfo>
HarnessPacks-->>User: Updated pack list (pack is untrusted)
end
rect rgba(144, 238, 144, 0.5)
Note over User,HarnessPacks: Trust and activate the pack
User->>HarnessPacks: Click Trust
HarnessPacks->>harness_backend: set_pack_trusted(id, true)
harness_backend->>Registry: update trusted flag, save registry.json
harness_backend-->>HarnessPacks: Vec<PackInfo>
User->>HarnessPacks: Click Activate
HarnessPacks->>harness_backend: set_active_pack(id)
harness_backend->>HarnessConfig: set active_pack, persist config
harness_backend-->>HarnessPacks: Vec<PackInfo>
HarnessPacks->>HarnessPage: increment reloadKey
HarnessPage->>HarnessSection: remount (refetch harness data from new pack)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src-tauri/src/harness.rs (2)
681-719:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftHonor manifest-declared blocks and units during validation and sync.
The manifest exposes
blocks/units, but validation and sync still require and apply only the hardcoded legacy component arrays. A pack declaring alternate block files or unit bundles inpharos-harness.jsonwill be rejected or ignored, breaking the pack manifest contract described by this PR.Also applies to: 733-754, 920-928
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/harness.rs` around lines 681 - 719, The apply_harness function and other sync/validation functions use hardcoded component arrays BLOCK_COMPONENTS and UNIT_COMPONENTS instead of respecting the manifest-declared blocks and units loaded from the configuration. Replace the iterations over these hardcoded arrays with iterations over the manifest-declared components obtained from the cfg object. Specifically, use the blocks and units declared in the loaded HarnessConfig rather than the legacy component arrays, and apply the same pattern to all affected functions mentioned in the validation and sync code paths (including the functions around lines 733-754 and 920-928).
468-476:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the active pack for unit item listings.
Line 470 still reads items from the builtin repo, so with an external active pack the unit-item UI lists/checks the wrong
skills/hooks/commandsentries and reportspresentagainst the wrong source path.Proposed fix
let cfg = HarnessConfig::load_from(&config_path()); let disabled = cfg.disabled_items(component); - let src = repo_dir().join("claude").join(component); + let src = pack_dir(&cfg).join(component); let dst = home_dir().join(".claude").join(component);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/harness.rs` around lines 468 - 476, The variable src on line 470 is hardcoded to use repo_dir(), which always points to the builtin repository regardless of whether an active pack is configured. Instead of always reading from repo_dir().join("claude").join(component), modify the code to check if an active pack is configured in the HarnessConfig object. If an active pack exists, use its directory path for src instead of the builtin repo_dir(). This ensures that unit_item_names and the subsequent present check via is_our_link_into will correctly reference items from the active pack when one is configured.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/harness.rs`:
- Around line 1079-1080: The temp directory path is based only on the process
ID, which causes conflicts when multiple concurrent clone operations run in the
same process. Replace the process ID-based naming in the format string with a
unique identifier per operation (such as a UUID generated using
uuid::Uuid::new_v4() or a similar approach) to ensure each clone operation has
its own isolated temp directory. This fix needs to be applied in both locations
where this temp directory pattern appears in the code.
- Around line 1007-1014: The unique_pack_id function's taken closure only checks
if pack directories exist in the file system, but does not verify registry
entries. This creates a security vulnerability where a stale trusted registry
entry can allow an untrusted pack to reuse a removed pack's ID. Modify the taken
closure to check both directory existence from packs_dir() and whether the pack
ID exists in the registry entries. This ensures that IDs allocated by
unique_pack_id are unique across both the file system and the registry,
preventing reuse of IDs that still have registry entries.
- Around line 1096-1107: In the set_pack_trusted_inner function, when trust is
being revoked (trusted is set to false) for a pack that is currently active, the
pack must be deactivated to prevent its hooks from remaining linked. After
updating the pack's trusted flag in the registry, check if the pack being
modified is the active pack and if trust is being revoked. If both conditions
are true, deactivate the active pack before saving the registry, ensuring the
trust gate protects the user after revocation.
- Around line 327-331: The load_or_builtin function currently catches all errors
from read_to_string with a blanket Err(_) => Ok(Self::builtin()) pattern, which
silently treats permission errors, invalid UTF-8, and other failures as "no
manifest exists". Instead, you need to distinguish between a missing manifest
file and other read errors. Check if the error kind is
std::io::ErrorKind::NotFound - only in that case should you fall back to
Self::builtin(). For all other error types (permission denied, invalid UTF-8,
etc.), propagate them as an error by converting the IO error to a String and
returning it as an Err result.
- Around line 1079-1084: In the git clone command construction where the URL
argument is passed in the args array, add the `--` option delimiter before the
user-supplied URL to prevent it from being interpreted as a git option. This
protects against argument injection when the URL starts with a dash character.
Locate where `.args(["clone", "--depth", "1", url])` is called and insert `--`
into the args array to mark the boundary between git options and positional
arguments before the URL.
In `@src/components/SettingsPanel.tsx`:
- Around line 366-373: The addLocal and addGit functions do not guard against
concurrent operations while busy is true. Add a guard clause at the beginning of
both the addLocal and addGit functions that checks if busy is true and returns
early if it is, preventing users from triggering overlapping add or clone
operations when an action is already running. This same guard pattern should
also be applied to any other functions in the range 404-423 that handle user
submissions.
- Around line 395-397: The trust revocation button for non-builtin packs should
not be allowed to execute if the pack is currently the active external pack, as
revoking trust without triggering a reload would leave third-party hooks active
or leave the HarnessSection stale. Add a guard condition to the button that
renders the untrustButton (the button with className "harness-pack-btn" that
calls trust(p.id, false)) to check whether pack p is the currently active pack,
and either disable the button or prevent it from rendering entirely if it is the
active pack. This ensures users cannot revoke trust for whichever pack is
currently loaded without a proper reload cycle.
---
Outside diff comments:
In `@src-tauri/src/harness.rs`:
- Around line 681-719: The apply_harness function and other sync/validation
functions use hardcoded component arrays BLOCK_COMPONENTS and UNIT_COMPONENTS
instead of respecting the manifest-declared blocks and units loaded from the
configuration. Replace the iterations over these hardcoded arrays with
iterations over the manifest-declared components obtained from the cfg object.
Specifically, use the blocks and units declared in the loaded HarnessConfig
rather than the legacy component arrays, and apply the same pattern to all
affected functions mentioned in the validation and sync code paths (including
the functions around lines 733-754 and 920-928).
- Around line 468-476: The variable src on line 470 is hardcoded to use
repo_dir(), which always points to the builtin repository regardless of whether
an active pack is configured. Instead of always reading from
repo_dir().join("claude").join(component), modify the code to check if an active
pack is configured in the HarnessConfig object. If an active pack exists, use
its directory path for src instead of the builtin repo_dir(). This ensures that
unit_item_names and the subsequent present check via is_our_link_into will
correctly reference items from the active pack when one is configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed7dd167-4b7b-4e27-b3d3-bb0e91ab59f1
📒 Files selected for processing (4)
src-tauri/src/harness.rssrc-tauri/src/lib.rssrc/components/SettingsPanel.csssrc/components/SettingsPanel.tsx
| pub fn load_or_builtin(dir: &Path) -> Result<Self, String> { | ||
| match std::fs::read_to_string(dir.join(MANIFEST_FILE)) { | ||
| Ok(s) => Self::parse(&s), | ||
| Err(_) => Ok(Self::builtin()), | ||
| } |
There was a problem hiding this comment.
Only fall back for a missing manifest.
Line 330 currently treats permission errors, invalid UTF-8, and other read failures as “no manifest” and silently reports the builtin manifest, despite the comment promising a hard error for present-but-bad manifests.
Proposed fix
pub fn load_or_builtin(dir: &Path) -> Result<Self, String> {
- match std::fs::read_to_string(dir.join(MANIFEST_FILE)) {
+ let path = dir.join(MANIFEST_FILE);
+ match std::fs::read_to_string(&path) {
Ok(s) => Self::parse(&s),
- Err(_) => Ok(Self::builtin()),
+ Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(Self::builtin()),
+ Err(e) => Err(format!("매니페스트 읽기 실패 ({}): {e}", path.display())),
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/harness.rs` around lines 327 - 331, The load_or_builtin
function currently catches all errors from read_to_string with a blanket Err(_)
=> Ok(Self::builtin()) pattern, which silently treats permission errors, invalid
UTF-8, and other failures as "no manifest exists". Instead, you need to
distinguish between a missing manifest file and other read errors. Check if the
error kind is std::io::ErrorKind::NotFound - only in that case should you fall
back to Self::builtin(). For all other error types (permission denied, invalid
UTF-8, etc.), propagate them as an error by converting the IO error to a String
and returning it as an Err result.
| let taken = |id: &str| packs_dir().join(id).exists(); | ||
| if !taken(stem) { | ||
| return stem.to_string(); | ||
| } | ||
| (2..) | ||
| .map(|n| format!("{stem}-{n}")) | ||
| .find(|id| !taken(id)) | ||
| .unwrap_or_else(|| stem.to_string()) |
There was a problem hiding this comment.
Include registry IDs when allocating pack IDs.
unique_pack_id only checks directories. If a trusted registry entry remains after its directory is removed, a newly added untrusted pack can reuse that ID; the trust check on Lines 984-985 then sees the stale trusted entry and allows activation.
Proposed fix
- let taken = |id: &str| packs_dir().join(id).exists();
+ let registry_ids: std::collections::HashSet<String> =
+ load_registry().into_iter().map(|m| m.id).collect();
+ let taken = |id: &str| packs_dir().join(id).exists() || registry_ids.contains(id);Also applies to: 981-987
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/harness.rs` around lines 1007 - 1014, The unique_pack_id
function's taken closure only checks if pack directories exist in the file
system, but does not verify registry entries. This creates a security
vulnerability where a stale trusted registry entry can allow an untrusted pack
to reuse a removed pack's ID. Modify the taken closure to check both directory
existence from packs_dir() and whether the pack ID exists in the registry
entries. This ensures that IDs allocated by unique_pack_id are unique across
both the file system and the registry, preventing reuse of IDs that still have
registry entries.
| let tmp = std::env::temp_dir().join(format!("pharos-clone-{}", std::process::id())); | ||
| let _ = std::fs::remove_dir_all(&tmp); |
There was a problem hiding this comment.
Use a unique clone temp directory per operation.
The temp path is based only on the process ID, so two concurrent clone requests in the same app process can delete or overwrite each other’s work.
Proposed fix
- let tmp = std::env::temp_dir().join(format!("pharos-clone-{}", std::process::id()));
+ let nonce = SystemTime::now()
+ .duration_since(UNIX_EPOCH)
+ .map(|d| d.as_nanos())
+ .unwrap_or(0);
+ let tmp = std::env::temp_dir().join(format!("pharos-clone-{}-{nonce}", std::process::id()));Also applies to: 1091-1092
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/harness.rs` around lines 1079 - 1080, The temp directory path
is based only on the process ID, which causes conflicts when multiple concurrent
clone operations run in the same process. Replace the process ID-based naming in
the format string with a unique identifier per operation (such as a UUID
generated using uuid::Uuid::new_v4() or a similar approach) to ensure each clone
operation has its own isolated temp directory. This fix needs to be applied in
both locations where this temp directory pattern appears in the code.
| let tmp = std::env::temp_dir().join(format!("pharos-clone-{}", std::process::id())); | ||
| let _ = std::fs::remove_dir_all(&tmp); | ||
| let out = std::process::Command::new("git") | ||
| .args(["clone", "--depth", "1", url]) | ||
| .arg(&tmp) | ||
| .output() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does git clone -- stop option parsing for repository arguments that begin with-?
💡 Result:
Yes, the double dash (--) separator in git clone -- stops option parsing, ensuring that subsequent arguments are treated as positional arguments (such as the repository URL and the target directory) rather than command-line options [1][2][3]. In Git's command-line interface, using -- is the standard way to signal the end of option flags [4]. This is particularly useful when a repository URL or a directory name begins with a dash (-), as it prevents Git from incorrectly interpreting those strings as command-line options [4]. Official documentation for git clone explicitly lists [--] in its synopsis, indicating that it is a supported mechanism for disambiguating arguments [1][3].
Citations:
- 1: https://git-scm.com/docs/git-clone
- 2: https://github.com/git/git/blob/e942292a3e099f03ae23efd3fd9f09f8b512cadf/Documentation/git-clone.txt
- 3: https://github.com/git/htmldocs/blob/gh-pages/git-clone.adoc
- 4: https://stackoverflow.com/questions/22750028/in-git-what-does-dash-dash-mean
Terminate git options before the user-supplied URL to prevent argument injection.
A URL beginning with - can be parsed by git clone as another option, allowing argument injection. The -- delimiter must be added before the repository argument to ensure the URL is treated as a positional argument.
Proposed fix
let out = std::process::Command::new("git")
- .args(["clone", "--depth", "1", url])
+ .args(["clone", "--depth", "1", "--"])
+ .arg(url)
.arg(&tmp)
.output()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let tmp = std::env::temp_dir().join(format!("pharos-clone-{}", std::process::id())); | |
| let _ = std::fs::remove_dir_all(&tmp); | |
| let out = std::process::Command::new("git") | |
| .args(["clone", "--depth", "1", url]) | |
| .arg(&tmp) | |
| .output() | |
| let tmp = std::env::temp_dir().join(format!("pharos-clone-{}", std::process::id())); | |
| let _ = std::fs::remove_dir_all(&tmp); | |
| let out = std::process::Command::new("git") | |
| .args(["clone", "--depth", "1", "--"]) | |
| .arg(url) | |
| .arg(&tmp) | |
| .output() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/harness.rs` around lines 1079 - 1084, In the git clone command
construction where the URL argument is passed in the args array, add the `--`
option delimiter before the user-supplied URL to prevent it from being
interpreted as a git option. This protects against argument injection when the
URL starts with a dash character. Locate where `.args(["clone", "--depth", "1",
url])` is called and insert `--` into the args array to mark the boundary
between git options and positional arguments before the URL.
| /// Flip a pack's trust flag. The builtin pack is always trusted. | ||
| fn set_pack_trusted_inner(id: &str, trusted: bool) -> Result<Vec<PackInfo>, String> { | ||
| if id == BUILTIN_PACK_ID { | ||
| return Err("builtin 팩의 신뢰 설정은 바꿀 수 없습니다.".to_string()); | ||
| } | ||
| let mut reg = load_registry(); | ||
| match reg.iter_mut().find(|m| m.id == id) { | ||
| Some(m) => m.trusted = trusted, | ||
| None => return Err(format!("레지스트리에 없는 팩: {id}")), | ||
| } | ||
| save_registry(®)?; | ||
| Ok(list_packs_inner()) |
There was a problem hiding this comment.
Deactivate the active pack when trust is revoked.
Revoking trust only flips the registry flag. If the pack is already active, its hooks remain linked and future syncs still resolve through cfg.active_pack, so the trust gate no longer protects the user after revocation.
Proposed fix
fn set_pack_trusted_inner(id: &str, trusted: bool) -> Result<Vec<PackInfo>, String> {
if id == BUILTIN_PACK_ID {
return Err("builtin 팩의 신뢰 설정은 바꿀 수 없습니다.".to_string());
}
+ let cfg_path = config_path();
+ let mut cfg = HarnessConfig::load_from(&cfg_path);
+ let was_active = cfg.active_pack.as_deref() == Some(id);
let mut reg = load_registry();
match reg.iter_mut().find(|m| m.id == id) {
Some(m) => m.trusted = trusted,
None => return Err(format!("레지스트리에 없는 팩: {id}")),
}
save_registry(®)?;
+ if was_active && !trusted {
+ cfg.active_pack = None;
+ cfg.save_to(&cfg_path).map_err(|e| format!("config 저장 실패: {e}"))?;
+ apply_harness()?;
+ }
Ok(list_packs_inner())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Flip a pack's trust flag. The builtin pack is always trusted. | |
| fn set_pack_trusted_inner(id: &str, trusted: bool) -> Result<Vec<PackInfo>, String> { | |
| if id == BUILTIN_PACK_ID { | |
| return Err("builtin 팩의 신뢰 설정은 바꿀 수 없습니다.".to_string()); | |
| } | |
| let mut reg = load_registry(); | |
| match reg.iter_mut().find(|m| m.id == id) { | |
| Some(m) => m.trusted = trusted, | |
| None => return Err(format!("레지스트리에 없는 팩: {id}")), | |
| } | |
| save_registry(®)?; | |
| Ok(list_packs_inner()) | |
| /// Flip a pack's trust flag. The builtin pack is always trusted. | |
| fn set_pack_trusted_inner(id: &str, trusted: bool) -> Result<Vec<PackInfo>, String> { | |
| if id == BUILTIN_PACK_ID { | |
| return Err("builtin 팩의 신뢰 설정은 바꿀 수 없습니다.".to_string()); | |
| } | |
| let cfg_path = config_path(); | |
| let mut cfg = HarnessConfig::load_from(&cfg_path); | |
| let was_active = cfg.active_pack.as_deref() == Some(id); | |
| let mut reg = load_registry(); | |
| match reg.iter_mut().find(|m| m.id == id) { | |
| Some(m) => m.trusted = trusted, | |
| None => return Err(format!("레지스트리에 없는 팩: {id}")), | |
| } | |
| save_registry(®)?; | |
| if was_active && !trusted { | |
| cfg.active_pack = None; | |
| cfg.save_to(&cfg_path).map_err(|e| format!("config 저장 실패: {e}"))?; | |
| apply_harness()?; | |
| } | |
| Ok(list_packs_inner()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/harness.rs` around lines 1096 - 1107, In the
set_pack_trusted_inner function, when trust is being revoked (trusted is set to
false) for a pack that is currently active, the pack must be deactivated to
prevent its hooks from remaining linked. After updating the pack's trusted flag
in the registry, check if the pack being modified is the active pack and if
trust is being revoked. If both conditions are true, deactivate the active pack
before saving the registry, ensuring the trust gate protects the user after
revocation.
| const addLocal = () => { | ||
| const path = addPath.trim(); | ||
| if (path) run(invoke<PackInfo[]>("add_harness_pack_from_path", { path }), () => setAddPath("")); | ||
| }; | ||
| const addGit = () => { | ||
| const url = gitUrl.trim(); | ||
| if (url) run(invoke<PackInfo[]>("add_harness_pack_from_git", { url }), () => setGitUrl("")); | ||
| }; |
There was a problem hiding this comment.
Block add/clone submissions while an action is already running.
The buttons are disabled with busy, but the inputs still accept Enter and addLocal/addGit do not check busy, so a user can start overlapping copy/clone + registry updates.
Suggested guard
const addLocal = () => {
+ if (busy) return;
const path = addPath.trim();
if (path) run(invoke<PackInfo[]>("add_harness_pack_from_path", { path }), () => setAddPath(""));
};
const addGit = () => {
+ if (busy) return;
const url = gitUrl.trim();
if (url) run(invoke<PackInfo[]>("add_harness_pack_from_git", { url }), () => setGitUrl(""));
};
@@
<input
className="settings-trust-input"
value={addPath}
onChange={(e) => setAddPath(e.target.value)}
placeholder="로컬 팩 폴더 경로 추가…"
onKeyDown={(e) => { if (e.key === "Enter") addLocal(); }}
+ disabled={busy}
/>
@@
<input
className="settings-trust-input"
value={gitUrl}
onChange={(e) => setGitUrl(e.target.value)}
placeholder="git URL로 clone… (미신뢰로 추가)"
onKeyDown={(e) => { if (e.key === "Enter") addGit(); }}
+ disabled={busy}
/>Also applies to: 404-423
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/SettingsPanel.tsx` around lines 366 - 373, The addLocal and
addGit functions do not guard against concurrent operations while busy is true.
Add a guard clause at the beginning of both the addLocal and addGit functions
that checks if busy is true and returns early if it is, preventing users from
triggering overlapping add or clone operations when an action is already
running. This same guard pattern should also be applied to any other functions
in the range 404-423 that handle user submissions.
| {!p.builtin && p.trusted && ( | ||
| <button className="harness-pack-btn" onClick={() => trust(p.id, false)} disabled={busy} title="신뢰 해제">🔒</button> | ||
| )} |
There was a problem hiding this comment.
Do not let the active external pack become untrusted in-place.
Line 396 allows revoking trust for an active non-builtin pack, while trust() does not trigger a pack switch reload. That can leave third-party hooks active after trust is revoked, or leave HarnessSection stale if the backend switches away.
Suggested guard
{!p.builtin && p.trusted && (
- <button className="harness-pack-btn" onClick={() => trust(p.id, false)} disabled={busy} title="신뢰 해제">🔒</button>
+ <button
+ className="harness-pack-btn"
+ onClick={() => trust(p.id, false)}
+ disabled={busy || p.active}
+ title={p.active ? "활성 팩은 먼저 다른 팩으로 전환하세요" : "신뢰 해제"}
+ aria-label={`Untrust ${p.name}`}
+ >
+ 🔒
+ </button>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {!p.builtin && p.trusted && ( | |
| <button className="harness-pack-btn" onClick={() => trust(p.id, false)} disabled={busy} title="신뢰 해제">🔒</button> | |
| )} | |
| {!p.builtin && p.trusted && ( | |
| <button | |
| className="harness-pack-btn" | |
| onClick={() => trust(p.id, false)} | |
| disabled={busy || p.active} | |
| title={p.active ? "활성 팩은 먼저 다른 팩으로 전환하세요" : "신뢰 해제"} | |
| aria-label={`Untrust ${p.name}`} | |
| > | |
| 🔒 | |
| </button> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/SettingsPanel.tsx` around lines 395 - 397, The trust
revocation button for non-builtin packs should not be allowed to execute if the
pack is currently the active external pack, as revoking trust without triggering
a reload would leave third-party hooks active or leave the HarnessSection stale.
Add a guard condition to the button that renders the untrustButton (the button
with className "harness-pack-btn" that calls trust(p.id, false)) to check
whether pack p is the currently active pack, and either disable the button or
prevent it from rendering entirely if it is the active pack. This ensures users
cannot revoke trust for whichever pack is currently loaded without a proper
reload cycle.
- update_harness_pack: 소스에서 재페치. 로컬 경로는 재복사(신뢰 유지), git은 재클론 후 trust 리셋(새 원격 코드) + 활성 시 builtin 복귀로 미검토 코드 미적용. - min_app_version: 팩이 더 높은 앱 버전을 요구하면 활성화 차단 (version_ge로 비교, suffix 무시). - UI: 비-builtin 팩에 "갱신" 버튼. - 테스트: min_app_version 차단, 로컬 update 재복사·신뢰 유지(20 passed). - PackInfo Debug 파생(unwrap_err용). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
경로 직접 입력 대신 OS 폴더 선택 다이얼로그로 로컬 팩 추가(@tauri-apps/ plugin-dialog open directory). capability에 dialog:allow-open 명시. "찾아보기" 버튼 → 선택 즉시 add_harness_pack_from_path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
요약
하네스를 **외부에서 받아 갈아치울 수 있는 "팩"**으로. 단일 고정 레포(
~/pharos/claude)에서 → 교체 가능한 팩 시스템으로 확장. 컴포넌트 구조는 불변(회귀 0), 소스만 갈아끼움.레이어
pack_dir()한 곳으로.active_pack(config)에 따라 builtin(pharos repo) 또는 레지스트리 팩을 가리킴. 매니페스트(pharos-harness.json) 디스크립터, builtin이 레거시 컴포넌트 재현(parity 테스트 가드).~/.config/pharos/harnesses/registry.json, 팩 dir 바깥 → trust 위조 불가). trust 게이트: builtin 외 팩은 신뢰해야 활성화(hooks = 제3자 코드). trust/untrust/remove.set_active_pack이 재적용(기존 per-item 마이그레이션/backup 가드 재사용). 스왑 시 매니저 리로드.UI (Settings → Harness)
Active Pack 매니저: 팩 목록(active/trusted/source) · 로컬 경로 추가 · git URL clone · 신뢰/해제/제거.
검증
cargo check+tsc+vitest588 passed, harness 유닛테스트 18 passed:보안
외부 팩 활성화 = 그 팩 hooks가 Claude Code에서 실행됨. git clone 팩은 기본 미신뢰, 사용자가 명시적으로 신뢰해야 활성화. trust는 팩 외부 레지스트리에 저장.
후속(이 PR 범위 밖)
git 팩 update/pull, min_app_version 강제, 네이티브 폴더 피커, 체크섬/서명.
🤖 Generated with Claude Code
Summary by CodeRabbit