Add implementation plan for universal harness access#722
Conversation
Review: #722Head SHA: 88c5bca SummaryThis PR adds ADR-0037 (universal harness access via URLs and paths), a comprehensive implementation plan, and a README update indexing the FindingsInfo
FooterOutcome: approve Previous runReview: #722Head SHA: 4abf3e7 SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs and paths, plus README updates to surface the FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (2)Review: #722Head SHA: 8778d1f SummaryThis PR adds two new design documents (ADR-0037 and an implementation plan) for universal harness access via URLs and paths, plus a README update linking existing plan docs. The documents are well-structured, thoroughly address security concerns (SSRF, TOCTOU, dependency confusion, prompt injection), and the illustrative Go code demonstrates sound defensive patterns (DNS rebinding protection, content-addressed caching, integrity verification). The change is documentation-only with no runtime impact. A few medium and low-severity observations are noted below regarding ADR numbering, code illustrations, and minor editorial items. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (3)Review: #722Head SHA: 6fe6d2d SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs and paths. The design is thorough — it presents multiple options with clear trade-offs, addresses security concerns comprehensively (SSRF, DNS rebinding, TOCTOU, dependency confusion, prompt injection), and proposes a sensible phased implementation. The ADR numbering is correct (follows 0036), all cross-referenced documents exist, and the README update is appropriate housekeeping. A few minor informational items are noted below but none are blocking. FindingsMedium(none) Low
Info
FooterOutcome: approve Previous run (4)Review: #722Head SHA: 74321e2 SummaryThis PR adds a well-structured ADR (0037) and implementation plan for universal harness access via URLs and paths. The design is thorough, with strong security analysis covering SSRF, TOCTOU, dependency confusion, and prompt injection threats. The decision to restrict executable resources (scripts/binaries) to local-only while allowing declarative resources (agents, skills, policies) via URL is sound and aligns with the repo's threat priority model. Three medium findings relate to internal consistency between the ADR and plan documents, and two issues in the illustrative Go code that should be corrected before the code samples are used as implementation guidance. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (5)Review: #722Head SHA: ad75dd5 SummaryThis PR adds ADR-0037 and a detailed implementation plan for universal harness access via URLs. The documents are thorough, well-structured, and the security analysis is comprehensive — covering SSRF, TOCTOU, dependency confusion, DNS rebinding, and prompt injection threats with appropriate mitigations. The change is documentation-only with no Go code modifications. Two minor quality issues were found: a duplicated paragraph in the ADR Decision section and the README change bundles links to 3 pre-existing plan files alongside the new one (not harmful but worth noting for commit hygiene). FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (6)Review: #722Head SHA: 3ae4aad SummaryThis PR adds ADR-0037 (universal harness access via URLs and paths), a comprehensive implementation plan, and README updates linking the new plan and three pre-existing plans. The design is thorough, with well-considered security mitigations (SSRF protection, mandatory hash pinning, DNS rebinding prevention, content-addressed caching). The code examples are illustrative and demonstrate sound security practices. Two findings are worth noting: the ADR declares itself "Accepted" while the PR body requests security review before acceptance, and the README change bundles index entries for three pre-existing plan documents alongside the new one. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (7)Review: #722Head SHA: c5b4086 SummaryThis is a well-structured documentation PR that adds a new ADR (0037, Proposed status) and a detailed implementation plan for universal harness access via URLs and paths. The PR follows project conventions: the ADR presents multiple options with trade-offs (Options A–D and Z), explicitly marks the decision as "Proposed — pending security review," and the implementation plan provides phased delivery with thorough security analysis. The README update correctly links to the new plan document and three pre-existing plan documents that were previously unlinked. All cross-referenced ADRs (0024, 0022, 0017) and issue (#776) exist and are relevant. No code changes are introduced — all Go code in the plan is illustrative. No critical or high findings. FindingsMedium
Low
Info
FooterOutcome: approve Previous run (8)Review: #722Head SHA: 5e7c2e6 SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs and paths. The documents are internally consistent, thoroughly analyze security implications (SSRF, TOCTOU, dependency confusion, prompt injection), and follow the repo's convention of presenting multiple options with trade-offs. The ADR is correctly marked "Proposed" with explicit gates before acceptance. No blocking findings — the design is sound and the documentation is high quality. FindingsCritical(none) High(none) Medium(none) Low
Info
FooterOutcome: approve Previous run (9)Review: #722Head SHA: 9175242 SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs and paths. The documents are thorough, internally consistent, and demonstrate careful security analysis across SSRF, TOCTOU, dependency confusion, and prompt injection vectors. The ADR correctly marks itself as "Proposed" with explicit gates before acceptance. Two non-blocking findings are noted: the new README FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (10)Review: #722Head SHA: 4a50255 SummaryThis is a well-structured documentation PR adding ADR-0037 and an implementation plan for universal harness access via URLs and paths. The ADR thoroughly analyzes security implications (SSRF, TOCTOU, dependency confusion, prompt injection) with concrete mitigations. The implementation plan includes detailed Go code sketches, a phased migration path, and a comprehensive testing strategy. No code changes are included — only documentation, a README index update, and a minor .gitignore housekeeping addition. The documents are internally consistent, reference existing ADRs correctly, and follow repo conventions. FindingsMedium
Low
Info
FooterOutcome: approve Previous run (11)Review: #722Head SHA: 067a668 SummaryThis PR adds a well-structured ADR (0037) and implementation plan for universal harness access via URLs, along with a README update documenting the FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (12)Review: #722Head SHA: 5e54842 SummaryThis is a well-constructed documentation PR adding ADR-0037 and a detailed implementation plan for universal harness access. The security analysis is thorough (SSRF, TOCTOU, dependency confusion, prompt injection), the phased migration is practical, and the code sketches are illustrative without being premature. A few minor issues are worth noting but none are blocking. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (13)Review: #722Head SHA: 5e54842 SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs, along with README updates to surface the existing FindingsMedium(none) Low
Info
FooterOutcome: comment-only Previous run (14)Review: #722Head SHA: c12fbab SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs and paths. The documents are thorough, correctly follow project conventions, and explicitly gate implementation behind security review. ADR numbering (0037) correctly follows the existing 0036. All cross-referenced ADRs (0017, 0022, 0024) and plan files exist. One minor out-of-scope change ( FindingsMedium / Low / Info
FooterOutcome: approve Previous run (15)Review: #722Head SHA: 7078c83 SummaryThis PR adds a well-structured ADR (0037) and implementation plan for universal harness access via URLs and paths. The documents follow repo conventions—presenting multiple options with trade-offs, maintaining open questions, and staying organization-agnostic. The security analysis is thorough: SSRF protection with DNS rebinding defense, mandatory hash pinning, content-addressed caching, domain allowlists, internal IP blocking, and no-redirect policy. The ADR is clearly marked "Proposed" and explicitly states it should not be implemented until security review is complete. A few minor observations are noted below but nothing blocks approval. FindingsMedium / Low / Info
FooterOutcome: approve Previous run (16)Review: #722Head SHA: 8cd3799 SummaryThis PR adds a well-structured proposed ADR (0037) and detailed implementation plan for universal harness access via URLs. The documents follow repo conventions (multiple options with trade-offs, open questions sections, "Proposed" status with explicit acceptance prerequisites). All cross-referenced documents (ADR-0022, ADR-0024, security-threat-model, agent-architecture) exist in the repo. The security analysis is thorough, covering SSRF, TOCTOU, dependency confusion, prompt injection, and DNS rebinding. No critical or high issues found. A few observations worth noting below. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (17)Review: #722Head SHA: 8c0a4e9 SummaryThis is a well-structured documentation PR adding an ADR (0037) and implementation plan for universal harness access via URLs and paths. The documents follow repo conventions — multiple options with trade-offs, open questions sections, and clear status gating ("Proposed" with explicit prerequisites before acceptance). The security analysis is thorough, covering SSRF protection, TOCTOU mitigation, dependency confusion, and prompt injection via remote resources. No executable code is changed. One minor scope observation noted below. FindingsLow
FooterOutcome: approve Previous run (18)Review: #722Head SHA: 3003b05 SummaryThis is a well-written documentation PR that adds a Proposed ADR (0037) and a detailed implementation plan for universal harness access via URLs. The documents are thorough, security-aware, and internally consistent. All referenced ADRs exist and the README link is correctly placed. One medium finding notes that the SSRF IP blocklist in the plan should be more comprehensive to prevent bypass when implemented. Two low/info items note an unrelated FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (19)Review: #722Head SHA: 665c4cd SummaryThis is a well-structured documentation PR that adds ADR-0037 (Proposed) and a detailed implementation plan for making harness resources accessible via URLs. The security analysis is thorough — SSRF protection, mandatory hash pinning, DNS rebinding defense, and the declarative-only restriction for remote resources are all well-reasoned. The design correctly rejects tiered trust in favor of uniform security. No critical or high issues found. A few minor observations are noted below. FindingsMedium
Low
Info
FooterOutcome: comment-only Previous run (20)Review: #722Head SHA: 4bafeab SummaryThis PR adds a well-structured ADR (0037) and detailed implementation plan for universal harness access via URLs and paths. The design documents are thorough, present multiple options with trade-offs (consistent with repo conventions), and include extensive security analysis covering SSRF, TOCTOU, dependency confusion, DNS rebinding, and prompt injection. The ADR is correctly marked as "Proposed" with explicit gates before acceptance. Two minor findings: an unrelated FindingsMedium
Low
Info
FooterOutcome: comment-only |
Fixes from review comment #4397770354: 1. Fix io.LimitReader truncation bug - read one extra byte and reject if response exceeds MaxSizeBytes instead of silently truncating 2. Add dependency resolution limits - MaxDepth (10) and MaxResources (50) to prevent runaway transitive dependency chains 3. Fix ADR numbering gap - rename ADR-0030 to ADR-0029 and update all references 4. Remove unrelated .gitignore change - remove 'fullsend' binary entry 5. Fix hardcoded audit log path - use ~/.cache/fullsend/audit/ instead of /var/log/fullsend/ 6. Add error check for os.UserHomeDir() call 7. Add missing import declarations (fmt, path/filepath) to audit package 8. Clarify HTTP/HTTPS classification in SSRF protection section Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ggallen
force-pushed
the
feat/universal-harness-access
branch
from
f2047b5 to
1314189
Compare
Site previewPreview: https://1160005f-site.fullsend-ai.workers.dev Commit: |
ggallen
changed the title
maruiz93
left a comment
maruiz93
left a comment
There was a problem hiding this comment.
I agree with the approach. The other day I mentioned this in the call that I really think external references make sense. There are a couple of things that we should acknowledge in the ADR though:
-
Barak mentioned that this is really similar to package management. But here we are not packaging the harnesses as they are a set of files that could actually be in different parts and not a blackbox downloaded from somewhere. I prefer this idea as it's more flexible and more likely to encourage people to reuse different pieces but there are cons like attack surface increased and need to check different things to download all the pieces that conforms the agent. We should at least mention this in the ADR.
-
This approach makes me think that it makes more sense to have our own set of harnes files and harness components in a separate github repo within the fullsend-ai. Maybe even more than one repo so it allows fullsend to provide different tiers of agents (first-class fully supported and evaluated) and second class but available for others and more likely to accept external suggestions and build community without being so strict with the evaluation .
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/u/271249281?s=60&v=4){kind=small}
Fixes from review comment #4397770354: 1. Fix io.LimitReader truncation bug - read one extra byte and reject if response exceeds MaxSizeBytes instead of silently truncating 2. Add dependency resolution limits - MaxDepth (10) and MaxResources (50) to prevent runaway transitive dependency chains 3. Fix ADR numbering gap - rename ADR-0030 to ADR-0029 and update all references 4. Remove unrelated .gitignore change - remove 'fullsend' binary entry 5. Fix hardcoded audit log path - use ~/.cache/fullsend/audit/ instead of /var/log/fullsend/ 6. Add error check for os.UserHomeDir() call 7. Add missing import declarations (fmt, path/filepath) to audit package 8. Clarify HTTP/HTTPS classification in SSRF protection section Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ggallen
force-pushed
the
feat/universal-harness-access
branch
from
749b70c to
2372441
Compare
ggallen
left a comment
ggallen
left a comment
There was a problem hiding this comment.
Review feedback addressed
I've addressed all the issues from the review:
1. Missing imports in implementation plan (Medium - Correctness)
Fixed the missing strings import in the resolve.go snippet. The url.go snippet already had all necessary imports.
2. Incomplete docs/plans/ index (Info - Style/conventions)
Added the missing plan files to README.md:
- ADR46 Drift Scanner
- Vertex Inference Provisioning
3. Cache persistence question (from @ralphbean)
Added clarification in the ADR's "Proposed approach" section about cache location and persistence.
Cache semantics: The cache is stored in the repository's workspace (e.g., .fullsend-cache/). In ephemeral CI/CD environments like GitHub Actions where each workflow run gets a fresh runner, the cache is rebuilt on each run. However, the implementation should leverage platform-native caching mechanisms (GitHub Actions cache, GitLab CI cache) to persist the content-addressed cache across workflow runs.
This means:
- First run: fetch remote resources and store in local cache, then upload to platform cache
- Subsequent runs: restore from platform cache, avoiding re-fetch unless resources have changed
This is similar to how dependency managers (npm, pip) work in CI - the dependencies are rebuilt each run, but restored from cache to avoid re-downloading.
|
fullsend review is working on this — view logs |
Added a "Trade-off" section to the implementation plan explaining that the "scripts must be local" requirement means the .fullsend repo will still contain script files, leading to potentially large diffs when scripts are updated. Proposed mitigations: - Minimal wrapper pattern: Local scripts become thin wrappers that download and verify URL-sourced scripts, minimizing diff size - Vendoring with lock files: Use a lock file approach where diffs show only URL+hash updates rather than full script content - Future sandboxing: If URL-sourced scripts are permitted later, they would run in heavily restricted sandboxes Addresses fullsend-ai#722 (comment) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
Changes based on ralphbean's review: - Change ADR status from Proposed to Accepted in frontmatter and body - Update Decision section to reflect acceptance following security review - Add cache integrity re-verification to TOCTOU mitigation section - Add preamble to Open Questions explaining intentional deferral - Clarify that open questions don't block core design acceptance Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
Revert status back to Proposed and provide concrete answers to all design questions: 1. Insider threat governance: Harness allowlists must be subset of org-level config.yaml allowlist (validated at load time) 2. Signature verification: Phase 1 uses hash pinning only; signatures deferred to Phase 3 as optional enhancement 3. Namespace governance: Decentralized publishing model; no central registry; consumers control trust via allowlists 4. Version resolution: No magic resolution; all references must be full URLs with explicit hashes 5. Offline mode: Support via --offline flag; fail if resources not cached; useful for air-gapped CI/CD 6. Lock file format: Phase 3 adds .fullsend/lock.yaml with full schema for transitive dependency pinning 7. URL scheme: Phase 1 supports bare https:// only; git+https:// structured refs deferred to Phase 2/3 All questions now have concrete decisions with rationale, enabling the ADR to move toward acceptance. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
Remove duplicated "Hybrid approach" heading that appeared twice consecutively in the Decision section. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
MEDIUM fixes: 1. ADR vs Plan inconsistency: Add note to Open Questions clarifying which questions are resolved in ADR-0037 vs truly open for implementation (top-level harness protection, cache eviction) 2. DNS rebinding IP handling: Document that production implementations should iterate through all validated IPs sequentially to handle IPv4/IPv6 network configurations 3. Wildcard domain matching: Fix *.example.com to match subdomains only (not bare domain), following TLS certificate conventions LOW fixes: 1. Remove unused "path" import from resolve.go example code Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
MEDIUM fix: - Update Consequences section header to reflect hybrid approach (was "Option A" but decision is hybrid A+C) LOW fix: - Document that .fullsend-cache/ should be added to .gitignore to prevent cache artifacts from being committed Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
Add notes to illustrative code examples flagging implementation concerns that should be addressed in production: 1. CachePut: Non-atomic writes (metadata.json, content written separately). Production should use atomic writes via temp file + rename to prevent partial cache entries. 2. isInternalIP: Repeated net.ParseCIDR allocations per call. Production should parse CIDR ranges once at package init and reuse them. These are acceptable for plan documents but worth documenting for implementers. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
MEDIUM fixes: 1. ADR numbering gap: Add note explaining 0029 and 0035 are intentionally reserved/skipped per project conventions 2. DNS rebinding protection: Update code to iterate through all validated IPs with fallback instead of pinning to ips[0] only 3. CIDR parsing performance: Move CIDR range parsing to package-level variables instead of parsing on every isInternalIP call LOW fixes: 1. Question organization: Move resolved questions (signature verification, namespace governance, version resolution) to new "Resolved Questions" section with [RESOLVED in ADR-0037] markers 2. Host header behavior: Add comment documenting that Host header uses original URL hostname (intentional for TLS SNI/cert validation) while DialContext pins to pre-validated IPs Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
MEDIUM fixes: 1. Default allowed domains: Remove cdn.fullsend.ai from DefaultPolicy and add note that default shipped config includes github.com/ fullsend-ai/library but has no special privilege (user-editable) 2. Phase annotations: Add phase markers to code sections to clearly delineate Phase 1 (single-level) vs Phase 2+ (transitive) features: - Added "Phase 1/2+" markers to skill resolution code - Added phase comments to depth limit checks - Marked cycle detection tests as Phase 2+ LOW fix: 1. Misleading SSRF examples: Replace cdn.fullsend.ai with generic placeholders (example.org, myorg) in allowed_domains and allowed_remote_resources examples to avoid implying preferential treatment Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
fullsend review is working on this — view logs |
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Thorough and well-designed ADR + implementation plan. The security analysis is strong, the phased rollout is pragmatic, and the uniform trust model is the right call. A few issues to address before merging — mostly consistency between the ADR prose and the pseudocode.
Findings summary:
- 4 requesting changes (cache key description, offline mode, resolver signature, cache permissions)
- 1 noted/deferred (SHA algorithm agility)
- 1 discussion point (script locality restriction)
See inline comments for details.
| 3. **Dependency confusion:** An attacker publishes a malicious skill at `https://attacker.com/skills/common-name` and tricks a harness into referencing it instead of the legitimate `https://fullsend.ai/skills/common-name`. **Mitigations:** | ||
| - Explicit URL references (no auto-resolution of names to URLs). | ||
| - User-controlled URL allowlists per organization (configurable in `config.yaml`). Fetches to URLs outside the allowlist are rejected. | ||
| - Mandatory hash pinning: The attacker cannot substitute content for an already-pinned URL. |
There was a problem hiding this comment.
[important] This line says "Every path field (agent, policy, skills[], host_files[].src, pre_script, post_script, etc.) accepts URLs" — but the Decision section (line 153) and the plan's resource type table explicitly restrict host_files, pre_script, post_script, and validation scripts to local-only.
This will confuse implementers. The sentence should exclude the local-only fields.
Discussion (non-blocking): More broadly — is the local-only restriction for scripts the right long-term call? The hash pinning + allowlist + CODEOWNERS protections that make remote declarative resources trustworthy apply equally to scripts. Sandboxing scripts doesn't really work either: post_script typically needs host access (push results, run validation, collect artifacts), so a sandbox with "no secrets, no network, no writes" makes it useless.
It's going to be really unsatisfying for users to define everything about an agent remotely except scripts — those still have to be copied everywhere. The "wall of text" diff problem on fullsend update that @ralphbean flagged remains unsolved for scripts.
Not blocking this PR on it, but worth a follow-up discussion on whether hash-pinned remote scripts (with the same allowlist/CODEOWNERS governance) should be permitted without sandboxing.
| 5. **Executable code from URLs:** Pre/post scripts fetched from URLs run on the runner host with full privileges. **Mitigation:** Apply **Option C** restriction: scripts and binaries must be local files. Only declarative resources (agents, skills, policies, schemas) can be URLs. **Alternative (future):** URL-sourced scripts could run in a restricted sandbox with no access to secrets, no network, and no filesystem writes outside `/tmp`. This requires designing an in-sandbox pre/post command execution mechanism (something like `pre_commands`/`post_commands` that run inside the sandbox before/after the agent's main execution). Today, `pre_script` and `post_script` run outside the sandbox. Any relaxation of the "scripts must be local" restriction depends on this prerequisite capability being implemented first. | ||
|
|
||
| 6. **Runtime dependency discovery increases attack surface:** If agents can fetch resources at runtime based on dynamic input (e.g., "I need a Python linting skill for this repo"), an attacker can manipulate input to trigger fetch of a malicious resource. **Mitigations:** | ||
| - Runtime resource loading is opt-in per harness (disabled by default). |
There was a problem hiding this comment.
[important] This says "The cache key is SHA256(URL + hash)" but the implementation plan's CachePath function uses only the content's SHA256 hash as the cache key:
func CachePath(workspaceRoot, hash string) string {
return filepath.Join(workspaceRoot, ".fullsend-cache", "resources", "sha256", hash)
}The implementation is correct — content-addressed storage means two URLs with identical content share a cache entry. The ADR text should match: the cache key is SHA256(content), not SHA256(URL + hash).
| } | ||
|
|
||
| logPath := filepath.Join(logDir, "fetches.jsonl") | ||
| f, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) |
There was a problem hiding this comment.
[important] This offline mode pseudocode rejects all remote references:
if offline && hasRemoteReferences(h) {
return fmt.Errorf("harness references remote resources but --offline is set")
}But both the ADR (line 310-311) and the plan prose (line 577) say offline mode should use cached resources and only fail on cache misses:
fullsend run --offline <harness>disables network fetches. If any required resource is not in cache, the run fails.
The pseudocode should pass an offline flag into the resolver, which attempts cache lookups and only errors on cache misses — not reject all remote references wholesale.
| if h.Policy != "" { | ||
| resolved.PolicyPath, err = resolveResourceWithLimits(ctx, workspaceRoot, h.Policy, h.AllowedRemoteResources, policy, 0, &resourceCount) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("resolving policy: %w", err) |
There was a problem hiding this comment.
[moderate] This function signature has no "parent context" parameter:
func resolveResourceWithLimits(ctx context.Context, workspaceRoot, ref string, ...)The design (lines 100-149) describes resolving relative paths against a parent URL's base path for URL-sourced resources (e.g., ../policies/rust-sandbox.yaml relative to the skill's URL). But there's no way to pass that parent URL here.
Phase 2 will require a breaking signature change. Worth adding a parentRef string parameter (unused in Phase 1) or a resolverState struct now so the Phase 1 implementation doesn't need a breaking change later.
| ```yaml | ||
| # Generated by fullsend lock harness/code.yaml | ||
| # DO NOT EDIT - This file is auto-generated | ||
| version: 1 |
There was a problem hiding this comment.
[moderate] Cache directories are created with 0755 and content files with 0644:
os.MkdirAll(dir, 0755)
os.WriteFile(..., content, 0644)On shared runners, any user can read cached policies and agent definitions. These may contain organizational configuration. Should use 0700/0600 to restrict access to the owning user.
| // Example: https://example.com/file.md#sha256=abc123... -> "abc123..." | ||
| // Returns an error if the hash is not a valid 64-character lowercase hex string. | ||
| func ParseIntegrityHash(rawURL string) (urlWithoutHash, hash string, hasHash bool) { | ||
| u, err := url.Parse(rawURL) |
There was a problem hiding this comment.
[moderate, deferred] The design hardcodes SHA-256 throughout — URL fragment format (#sha256=...), hash validation (64 hex chars), cache paths (sha256/). SHA-256 is still secure, but since this is greenfield design with no backward compatibility constraints, consider supporting algorithm-agile prefixes from day one (e.g., #sha512=... as a future option). The URL fragment already namespaces with sha256=, so the extensibility point is there — ParseIntegrityHash and CachePath would just need to handle the algorithm prefix. This avoids a painful migration if the project ever needs to upgrade hash algorithms.
4 participants
Proposes making harnesses and agents universally accessible via HTTP(S) URLs, absolute paths, or relative paths. This enables community sharing, decentralized evolution of agent capabilities, and runtime adaptation.
Key features:
Security concerns prominently documented:
Implementation changes detailed in
docs/plans/universal-harness-access.mdcovering resource fetcher, dependency resolver, cache layer, audit logging, and migration path across four phases.Additional changes:
docs/plans/section with links to universal-harness-access plan (new) and pre-existing plan files (agent-execution-environment, vertex-inference-provisioning, drift-scanner) for complete documentation indexStatus: Proposed (all design questions answered, ready for acceptance review)