Skip to content

release: v0.8.0-alpha.1 — RFC alignment, threat-model fixes, scoped caches, AX-6/AX-10#4

Open
Snider wants to merge 5 commits intomainfrom
release/v0.8.0-alpha.1
Open

release: v0.8.0-alpha.1 — RFC alignment, threat-model fixes, scoped caches, AX-6/AX-10#4
Snider wants to merge 5 commits intomainfrom
release/v0.8.0-alpha.1

Conversation

@Snider
Copy link
Copy Markdown
Contributor

@Snider Snider commented Apr 25, 2026

Summary

Consolidates the v0.8.0-alpha.1 release work for core/go-cache. Substantial RFC alignment pass, hardening across the cache surface, scoped caches, and threat-model audit fixes from Cerberus.

Headline changes

  • Module path migrationforge.lthn.ai/core/go-cachedappco.re/go/cache. All sibling dappco.re/go/* deps pinned to v0.8.0-alpha.1. Stale local replace directive removed; stale core/io direct dep migrated.
  • AX-10 scaffoldtests/cli/cache/Taskfile.yaml + driver coverage for the cache CLI surface.
  • Threat-model audit fixes (Cerberus, Mantis #923) —
    • pattern length validation
    • symlink escape prevention in cache base directory resolution
  • Cache RFC alignment
    • storage field naming standardised
    • HTTP cache request validation tightened
    • HTTP cache body paths hardened
    • response body path validation aligned with RFC contract
    • cache lifecycle and examples clarified per AX
    • cache module path documentation aligned
  • Scoped caches
    • ScopedCache.Scoped wrapper for nested scope composition
    • scoped clear-scope passthrough
    • invalidation patterns scoped to the originating cache
    • scoped cache key handling tightened
    • scoped cache clearing edge cases corrected
  • Hardening — cache metadata validation (corruption → miss, not error), cache namespace validation, cache serialization writes, cache deletion semantics, cache glob invalidation, cache HTTP storage APIs, cache registries and body paths, HTTP cache metadata validation (legacy + current), GitHub cache key segments escaped against unsafe values, HTTP cache storage now uses hashed keys, nil invalidation callbacks ignored, dotted cache storage names supported, preflight batch deletes, entries preserved on write failures.
  • Coverage — missing branches, contract coverage, RFC traversal, unit tests across the cache surface (multiple passes).
  • Documentation — cache HTTP examples tightened, cache API usage examples improved, cache docs synced with implemented API.

Refs

  • RFC.go-cache.md (RFC alignment + traversal contract)
  • RFC-CORE-008-AGENT-EXPERIENCE.md (AX-1, AX-6, AX-10)
  • Mantis #923 (threat-model audit, including high-severity symlink finding fix)

Test plan

  • go test ./... passes
  • tests/cli/cache Taskfile drivers exercise the CLI surface
  • CodeRabbit review

Co-authored-by: Athena athena@lthn.ai
Co-authored-by: Cerberus cerberus@lthn.ai
Co-authored-by: Hephaestus hephaestus@lthn.ai
Co-authored-by: Cladius Maximus cladius@lthn.ai

Summary by CodeRabbit

Release Notes

  • New Features

    • Added binary payload caching and retrieval alongside JSON entries
    • Added configurable time-to-live (TTL) per cache entry
    • Added scoped cache namespacing by origin
    • Added trigger-based cache invalidation system
    • Added HTTP cache storage API emulation
  • Security

    • Enhanced key validation with strict input checking
    • Added path traversal and symlink escape protections
  • Documentation

    • Updated module documentation and API contract
    • Added security threat model assessment

Snider and others added 5 commits April 7, 2026 14:58
Upgrade io v0.2.0→v0.4.1, log v0.0.4→v0.1.2 (forge path → dappco.re).

Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
…re primitives

Replaced fmt, strings, sort, os, io, sync, encoding/json, path/filepath,
errors, log, reflect with core.Sprintf, core.E, core.Contains, core.Trim,
core.Split, core.Join, core.JoinPath, slices.Sort, c.Fs(), c.Lock(),
core.JSONMarshal, core.ReadAll and other CoreGO v0.8.0 primitives.

Framework boundary exceptions preserved where stdlib types are required
by external interfaces (Gin, net/http, CGo, Wails, bubbletea).

Co-Authored-By: Virgil <virgil@lethean.io>
Spark draft + opus cleanup. Two real bugs fixed:

- collectJSONKeys("") built listing path as baseDir+"/" which the
  MockMedium.List doubled to "//", silently returning zero matches.
  Every pattern-based op iterated an empty key set
- matchKeyPattern used path.Match where * does not cross /. Per RFC
  §12.4 "dns/*" must match "dns/example.com/A" recursively. Rewrote
  to treat trailing /* as prefix semantics (any depth) plus per-
  segment glob for middle wildcards like "dns/charon.*"

Plus AX cleanup of spark's stdlib reintroductions (strings, sort,
path) — swapped to core.* primitives (Contains, HasPrefix, HasSuffix,
TrimSuffix, Split, SplitN, Trim, JoinPath) and slices.Sort.

Feature work preserved:
- scoped caches, invalidation triggers, HTTP CacheStorage, binary
  data support, per-key TTL
- ensureSafeCacheName extracted to DRY CacheStorage.Open/Delete
- All 20 tests pass

Co-Authored-By: Virgil <virgil@lethean.io>
…aches, AX-6/AX-10

Consolidates the v0.8.0-alpha.1 release work for core/go-cache. Substantial
RFC alignment pass, hardening across the cache surface, scoped caches, and
threat-model audit fixes from Cerberus.

* Module path migration: forge.lthn.ai/core/go-cache ->
  dappco.re/go/cache. All sibling dappco.re/go/* deps pinned to
  v0.8.0-alpha.1; stale local replace directive removed; stale
  core/io direct dep migrated.

* AX-10 scaffold: tests/cli/cache/Taskfile.yaml + driver coverage
  for the cache CLI surface.

* Threat-model audit fixes (Cerberus, Mantis #923):
    - pattern length validation
    - symlink escape prevention in cache base directory resolution

* Cache RFC alignment (multiple verification + alignment passes):
    - storage field naming standardised
    - HTTP cache request validation tightened
    - HTTP cache body paths hardened
    - response body path validation aligned with RFC contract
    - cache lifecycle and examples clarified per AX
    - cache module path documentation aligned
    - per-file inline tests + zero-value handling

* Scoped caches:
    - ScopedCache.Scoped wrapper for nested scope composition
    - scoped clear-scope passthrough
    - invalidation patterns scoped to the originating cache
    - scoped cache key handling tightened
    - scoped cache clearing edge cases corrected

* Hardening:
    - cache metadata validation (corruption -> miss, not error)
    - cache namespace validation
    - cache serialization writes
    - cache deletion semantics
    - cache glob invalidation
    - cache HTTP storage APIs
    - cache registries and body paths
    - HTTP cache metadata validation (legacy + current)
    - GitHub cache key segments escaped against unsafe values
    - HTTP cache storage now uses hashed keys
    - nil invalidation callbacks ignored
    - dotted cache storage names supported
    - preflight batch deletes
    - entries preserved on write failures
    - entry payload type aligned

* Coverage: missing branches, contract coverage, RFC traversal,
  unit tests across the cache surface (multiple passes).

* Documentation: cache HTTP examples tightened, cache API usage
  examples improved, cache docs synced with implemented API.

Refs: RFC.go-cache.md (RFC alignment + traversal contract)
      RFC-CORE-008-AGENT-EXPERIENCE.md (AX-1, AX-6, AX-10)
      Mantis #923 (threat-model audit)

Co-authored-by: Athena <athena@lthn.ai>
Co-authored-by: Cerberus <cerberus@lthn.ai>
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Co-authored-by: Cladius Maximus <cladius@lthn.ai>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

📝 Walkthrough

Walkthrough

This pull request reorganises the cache module from dappco.re/go/core/cache to dappco.re/go/cache, significantly expands the cache implementation with binary payload support, TTL management, trigger-based invalidation, scoped namespacing, and HTTP cache storage APIs, and adds comprehensive test coverage and threat modelling documentation.

Changes

Cohort / File(s) Summary
Module Path Refactoring
CLAUDE.md, go.mod
Updated module path from dappco.re/go/core/cache to dappco.re/go/cache and refreshed dependency to dappco.re/go/io v0.8.0-alpha.1 whilst removing indirect dappco.re/go/core/log dependency.
Core Cache Implementation
cache.go
Added binary caching with JSON metadata sidecars and .bin payloads, per-entry TTL support, scoped namespacing via origin hashing, trigger-based invalidation with glob pattern matching, strict input validation (length/control bytes/symlink/traversal checks), error propagation for malformed entries, and HTTP cache storage API emulation (CacheStorage, HTTPCache, CachedRequest/CachedResponse types).
Test Suite Expansion
cache_test.go
Introduced error-injection medium wrapper, helpers for HTTP-cache metadata keying (SHA-256 hex and Base64 RawURLEncoding formats), expanded test coverage for TTL validation, key/path safety with symlink defences, binary caching semantics, HTTP cache storage operations, transactional rollback on write failures, and metadata tampering rejection.
API & Architecture Documentation
docs/api-contract.md, docs/architecture.md
Updated exported symbol listings and API contract for new module path; added documentation for TTL methods, binary caching with sidecar format, scoped cache namespacing, invalidation callbacks, and HTTP cache storage abstractions.
General Documentation & Dependencies
docs/development.md, docs/index.md, docs/security-attack-vector-mapping.md
Refreshed import paths from forge.lthn.ai to dappco.re equivalents; updated module and scope identifiers throughout.
CLI Testing & Threat Modelling
tests/cli/cache/Taskfile.yaml, tests/cli/cache/main.go, threats.md
Added new CLI test driver performing cache round-trip validation via in-memory mock medium; introduced threat-model audit documenting glob-pattern length backstop gap (medium severity), symlink-following path traversal risk (high severity), and TOCTOU/eviction concerns (placeholders).
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.40% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title comprehensively covers the main changes: module path/RFC alignment, threat-model fixes, scoped caches, and AX compliance items (AX-6/AX-10).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (3)
tests/cli/cache/main.go (1)

19-37: Add minimal stderr diagnostics before exiting.

The exit codes are useful, but Line 19 onward currently drops the underlying error context, which makes failed CI runs harder to debug.

🔍 Minimal observability improvement
 import (
+	"fmt"
 	"os"
@@
 	c, err := cache.New(medium, "/cache", cache.DefaultTTL)
 	if err != nil {
+		fmt.Fprintf(os.Stderr, "cache.New failed: %v\n", err)
 		os.Exit(1)
 	}
@@
 	if err := c.Set("driver/roundtrip", payload); err != nil {
+		fmt.Fprintf(os.Stderr, "cache.Set failed: %v\n", err)
 		os.Exit(2)
 	}
@@
 	found, err := c.Get("driver/roundtrip", &out)
 	if err != nil {
+		fmt.Fprintf(os.Stderr, "cache.Get failed: %v\n", err)
 		os.Exit(3)
 	}
 	if !found {
+		fmt.Fprintln(os.Stderr, "cache.Get returned miss for driver/roundtrip")
 		os.Exit(4)
 	}
 	if out["hello"] != "world" {
+		fmt.Fprintf(os.Stderr, "unexpected payload: %#v\n", out)
 		os.Exit(5)
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli/cache/main.go` around lines 19 - 37, Before each os.Exit call in
the test (the error checks around c.Set("driver/roundtrip", payload),
c.Get("driver/roundtrip", &out), the found check, and the out["hello"] check)
write a minimal diagnostic to stderr including the operation and the underlying
error or state; for example, print the error returned by c.Set or c.Get (when
err != nil), and for the found/out checks print descriptive messages showing
found value or the actual out map contents, then call os.Exit with the existing
exit code—update the blocks that reference payload, c.Set, c.Get, found, and out
to emit these stderr diagnostics before exiting.
cache.go (1)

1671-1677: Silent error handling may hide data corruption.

When readResponseRecord fails (e.g., malformed JSON), the error is silently ignored and the entry is skipped. While this prevents one corrupt file from blocking enumeration, it's inconsistent with Get/Match which return errors for malformed entries. Consider logging at debug level or returning a partial result with an error indicator.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cache.go` around lines 1671 - 1677, The loop calling
httpCache.readResponseRecord currently swallows errors which can hide corrupt
entries; update the logic in the enumeration routine that iterates keys so that
when readResponseRecord(key) returns a non-nil err you do not silently ignore
it—either emit a debug-level log via the existing logger with the key and error
(e.g., include "httpCache.readResponseRecord", key, err) or accumulate the error
into a partial-result error list and return/propagate it alongside the
successful records (consistent with how Get/Match surface malformed-entry
errors). Ensure you still continue iteration after logging/recording the error
so one corrupt file doesn’t abort enumeration.
cache_test.go (1)

2271-2283: Header generation may produce fewer than 129 unique keys.

The key generation formula "X-Test-" + (i%26) + "-" + ((i/26)%10) produces duplicates when i >= 260 isn't reached, but within 129 iterations there are still collisions. For example, i=0 and i=260 would both produce "X-Test-a-0", but since you only iterate 129 times, the pattern cycles through 26 letters × 5 digits (0-4) = 130 unique combinations before repeating. With 129 iterations, you get 129 unique keys, which should work.

However, a cleaner approach would avoid the modular arithmetic complexity:

♻️ Suggested simplification
-		headers[core.Concat("X-Test-", string(rune('a'+(i%26))), "-", string(rune('0'+((i/26)%10))))] = "value"
+		headers[core.Concat("X-Header-", core.Itoa(i))] = "value"

This requires core.Itoa or strconv.Itoa, but guarantees unique keys without mental arithmetic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cache_test.go` around lines 2271 - 2283, The header key generation inside the
"too-many-headers" test's resp builder can produce confusing collisions; replace
the modular letter/digit math used in the headers map population (the loop that
sets headers[core.Concat(...)] = "value") with a simple, unique key per
iteration such as using strconv.Itoa(i) or fmt.Sprintf("X-Test-%d", i) so each
of the 129 entries is guaranteed unique; keep the map capacity and the
cache.CachedResponse construction the same but build keys from the loop index
instead of the current core.Concat expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cache.go`:
- Around line 814-826: rejectSymlink currently calls os.Lstat directly
(bypassing the io.Medium abstraction) which violates the I/O layering used by
Cache.Path/ensureNoSymlinkPath; add a new Lstat (or Stat that returns FileInfo
and error) method to the io.Medium interface, implement it in the real medium
and MockMedium, then replace os.Lstat usage in rejectSymlink (and any callers
like ensureNoSymlinkPath) with medium.Lstat so symlink checks go through
io.Medium and tests can simulate symlink behavior via MockMedium.

In `@docs/architecture.md`:
- Around line 162-164: The docs incorrectly state that Scoped(origin) hashes the
origin with SHA-1; update the documentation text to state SHA-256 to match the
implementation which calls sha256.Sum256 (referencing the Scoped(origin)
behavior and the use of sha256.Sum256 in cache.go) so the architecture.md
description and examples reflect SHA-256 as the namespace hashing algorithm.

In `@docs/index.md`:
- Around line 69-74: The docs table entry for `dappco.re/go/core/io` currently
lists `v0.4.1` and is out of sync with go.mod; update the docs/index.md module
table so it reflects the actual modules and versions declared in go.mod by
replacing the single `dappco.re/go/core/io v0.4.1` row with the correct rows and
versions `dappco.re/go/core v0.8.0-alpha.1` and `dappco.re/go/io v0.8.0-alpha.1`
(or otherwise match go.mod), ensuring the module names and version strings shown
in the table match the go.mod entries exactly.

In `@tests/cli/cache/Taskfile.yaml`:
- Around line 7-9: Replace the fixed /tmp binary path and brittle cleanup with a
unique temporary file and guaranteed teardown: create a temp path with mktemp
(or similar) instead of /tmp/go-cache-cli-driver, build to that temp path
(replace the literal "-o /tmp/go-cache-cli-driver"), run the binary from that
temp path, and ensure removal happens even if the command fails by registering a
trap or finally-style cleanup that removes the temp binary; update the three
commands in the Taskfile that reference "/tmp/go-cache-cli-driver" accordingly.

In `@threats.md`:
- Around line 23-26: Update the eviction/TOCTOU block in threats.md by replacing
the four "TBD" placeholders under "Finding/Severity/Repro test/Fix" with a
tracked outcome: either (A) a completed entry that identifies the root cause,
severity, repro steps, and a concrete remediation plan, or (B) an explicit
"out-of-scope" statement that links to a tracking issue (create one if missing)
and a short rationale; ensure the entry references "eviction/TOCTOU" and, if
applicable, follow the suggested Section 3 template (race surfaces, repro
matrix, fix checklist) or point to the linked issue for that template.

---

Nitpick comments:
In `@cache_test.go`:
- Around line 2271-2283: The header key generation inside the "too-many-headers"
test's resp builder can produce confusing collisions; replace the modular
letter/digit math used in the headers map population (the loop that sets
headers[core.Concat(...)] = "value") with a simple, unique key per iteration
such as using strconv.Itoa(i) or fmt.Sprintf("X-Test-%d", i) so each of the 129
entries is guaranteed unique; keep the map capacity and the cache.CachedResponse
construction the same but build keys from the loop index instead of the current
core.Concat expression.

In `@cache.go`:
- Around line 1671-1677: The loop calling httpCache.readResponseRecord currently
swallows errors which can hide corrupt entries; update the logic in the
enumeration routine that iterates keys so that when readResponseRecord(key)
returns a non-nil err you do not silently ignore it—either emit a debug-level
log via the existing logger with the key and error (e.g., include
"httpCache.readResponseRecord", key, err) or accumulate the error into a
partial-result error list and return/propagate it alongside the successful
records (consistent with how Get/Match surface malformed-entry errors). Ensure
you still continue iteration after logging/recording the error so one corrupt
file doesn’t abort enumeration.

In `@tests/cli/cache/main.go`:
- Around line 19-37: Before each os.Exit call in the test (the error checks
around c.Set("driver/roundtrip", payload), c.Get("driver/roundtrip", &out), the
found check, and the out["hello"] check) write a minimal diagnostic to stderr
including the operation and the underlying error or state; for example, print
the error returned by c.Set or c.Get (when err != nil), and for the found/out
checks print descriptive messages showing found value or the actual out map
contents, then call os.Exit with the existing exit code—update the blocks that
reference payload, c.Set, c.Get, found, and out to emit these stderr diagnostics
before exiting.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3f7264a7-843a-484b-83bb-b8cf4cc95fce

📥 Commits

Reviewing files that changed from the base of the PR and between 2ad3b05 and 0ef61b6.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (12)
  • CLAUDE.md
  • cache.go
  • cache_test.go
  • docs/api-contract.md
  • docs/architecture.md
  • docs/development.md
  • docs/index.md
  • docs/security-attack-vector-mapping.md
  • go.mod
  • tests/cli/cache/Taskfile.yaml
  • tests/cli/cache/main.go
  • threats.md

Comment thread cache.go
Comment on lines +814 to +826
func rejectSymlink(path string) error {
info, err := os.Lstat(path)
if err != nil {
if os.IsNotExist(err) {
return nil
}
return err
}
if info.Mode()&os.ModeSymlink != 0 {
return core.E("cache.validatePath", "path contains symlink", nil)
}
return nil
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if io.Medium interface has stat-like methods
ast-grep --pattern 'type Medium interface {
  $$$
}'

Repository: dAppCore/go-cache

Length of output: 43


🏁 Script executed:

# Search for io.Medium interface definition
rg "type Medium interface" -A 15

Repository: dAppCore/go-cache

Length of output: 43


🏁 Script executed:

# Check imports in cache.go to see if io.Medium is used elsewhere
rg "io\.Medium|dappco\.re/go/core/io" cache.go -B 2 -A 2

Repository: dAppCore/go-cache

Length of output: 1329


🏁 Script executed:

# Find all usages of rejectSymlink
rg "rejectSymlink" -B 3 -A 3

Repository: dAppCore/go-cache

Length of output: 630


🏁 Script executed:

# Check if there's an io package in the codebase
fd -type f -name "*.go" | xargs grep -l "package io" | head -5

Repository: dAppCore/go-cache

Length of output: 232


🏁 Script executed:

# Find Cache.Path() method implementation
rg "func \(.*\) Path\(" cache.go -A 15

Repository: dAppCore/go-cache

Length of output: 1224


🏁 Script executed:

# Check the ensureNoSymlinkPath function and its context
rg "func ensureNoSymlinkPath" -A 20

Repository: dAppCore/go-cache

Length of output: 729


🏁 Script executed:

# Look for documentation or comments about symlink handling
rg "symlink|Symlink" cache.go -B 2 -A 2

Repository: dAppCore/go-cache

Length of output: 768


🏁 Script executed:

# Check if rejectSymlink is called during public API calls
rg "ensureNoSymlinkPath" -B 5 -A 2

Repository: dAppCore/go-cache

Length of output: 793


os.Lstat in rejectSymlink bypasses the io.Medium abstraction.

The rejectSymlink function (lines 814–826) calls os.Lstat directly within ensureNoSymlinkPath, which is invoked from Cache.Path() for path traversal protection. This violates the guideline that all I/O must be delegated to the io.Medium interface.

Whilst this symlink validation serves as a critical security boundary check before any data operations, delegating it to io.Medium would:

  • Allow MockMedium to simulate symlink scenarios in tests
  • Maintain consistency with the abstraction layer across all filesystem operations
  • Preserve the security check regardless of the underlying storage backend

Consider adding an Lstat or Stat method to the io.Medium interface, or document this as an intentional security-boundary exception.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cache.go` around lines 814 - 826, rejectSymlink currently calls os.Lstat
directly (bypassing the io.Medium abstraction) which violates the I/O layering
used by Cache.Path/ensureNoSymlinkPath; add a new Lstat (or Stat that returns
FileInfo and error) method to the io.Medium interface, implement it in the real
medium and MockMedium, then replace os.Lstat usage in rejectSymlink (and any
callers like ensureNoSymlinkPath) with medium.Lstat so symlink checks go through
io.Medium and tests can simulate symlink behavior via MockMedium.

Comment thread docs/architecture.md
Comment on lines +162 to +164
`Scoped(origin)` creates a lightweight wrapper around the parent cache. The
wrapper hashes the origin with SHA-1 and uses the result as a fixed namespace
prefix:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Documentation states SHA-1 but implementation uses SHA-256.

The documentation says the wrapper "hashes the origin with SHA-1", but cache.go (line 877) uses sha256.Sum256. Update the documentation to reflect the actual algorithm.

-wrapper hashes the origin with SHA-1 and uses the result as a fixed namespace
+wrapper hashes the origin with SHA-256 and uses the result as a fixed namespace
📝 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.

Suggested change
`Scoped(origin)` creates a lightweight wrapper around the parent cache. The
wrapper hashes the origin with SHA-1 and uses the result as a fixed namespace
prefix:
`Scoped(origin)` creates a lightweight wrapper around the parent cache. The
wrapper hashes the origin with SHA-256 and uses the result as a fixed namespace
prefix:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/architecture.md` around lines 162 - 164, The docs incorrectly state that
Scoped(origin) hashes the origin with SHA-1; update the documentation text to
state SHA-256 to match the implementation which calls sha256.Sum256 (referencing
the Scoped(origin) behavior and the use of sha256.Sum256 in cache.go) so the
architecture.md description and examples reflect SHA-256 as the namespace
hashing algorithm.

Comment thread docs/index.md
Comment on lines 69 to 74
| Module | Version | Role |
|-------------------------------|---------|---------------------------------------------|
| `forge.lthn.ai/core/go-io` | v0.0.3 | Storage abstraction (`Medium` interface) |
| `forge.lthn.ai/core/go-log` | v0.0.1 | Structured logging (indirect, via `go-io`) |
| `dappco.re/go/core/io` | v0.4.1 | Storage abstraction (`Medium` interface) |

There are no other runtime dependencies. The test suite uses the standard
library only (plus the `MockMedium` from `go-io`).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== go.mod =="
sed -n '1,40p' go.mod

echo
echo "== docs/index.md dependency section =="
rg -n -A6 -B2 '## Dependencies|dappco\.re/go/' docs/index.md

Repository: dAppCore/go-cache

Length of output: 1123


Update dependency table in docs/index.md to match go.mod.

Line 71 documents dappco.re/go/core/io v0.4.1, but go.mod declares dappco.re/go/core v0.8.0-alpha.1 and dappco.re/go/io v0.8.0-alpha.1. The documentation will misdirect consumers to outdated modules.

📝 Suggested doc correction
 | Module                        | Version | Role                                       |
 |-------------------------------|---------|---------------------------------------------|
-| `dappco.re/go/core/io`   | v0.4.1  | Storage abstraction (`Medium` interface)    |
+| `dappco.re/go/io`        | v0.8.0-alpha.1 | Storage abstraction (`Medium` interface) |
+| `dappco.re/go/core`      | v0.8.0-alpha.1 | Core error/utility primitives           |
📝 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.

Suggested change
| Module | Version | Role |
|-------------------------------|---------|---------------------------------------------|
| `forge.lthn.ai/core/go-io` | v0.0.3 | Storage abstraction (`Medium` interface) |
| `forge.lthn.ai/core/go-log` | v0.0.1 | Structured logging (indirect, via `go-io`) |
| `dappco.re/go/core/io` | v0.4.1 | Storage abstraction (`Medium` interface) |
There are no other runtime dependencies. The test suite uses the standard
library only (plus the `MockMedium` from `go-io`).
| Module | Version | Role |
|-------------------------------|---------|---------------------------------------------|
| `dappco.re/go/io` | v0.8.0-alpha.1 | Storage abstraction (`Medium` interface) |
| `dappco.re/go/core` | v0.8.0-alpha.1 | Core error/utility primitives |
There are no other runtime dependencies. The test suite uses the standard
library only (plus the `MockMedium` from `go-io`).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/index.md` around lines 69 - 74, The docs table entry for
`dappco.re/go/core/io` currently lists `v0.4.1` and is out of sync with go.mod;
update the docs/index.md module table so it reflects the actual modules and
versions declared in go.mod by replacing the single `dappco.re/go/core/io
v0.4.1` row with the correct rows and versions `dappco.re/go/core
v0.8.0-alpha.1` and `dappco.re/go/io v0.8.0-alpha.1` (or otherwise match
go.mod), ensuring the module names and version strings shown in the table match
the go.mod entries exactly.

Comment on lines +7 to +9
- go build -o /tmp/go-cache-cli-driver ./tests/cli/cache
- /tmp/go-cache-cli-driver
- rm -f /tmp/go-cache-cli-driver
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid a fixed binary path in /tmp for the CLI driver.

Using one global path can cause collisions and flaky runs when tasks execute concurrently; cleanup also does not run if an earlier command fails.

🔧 Safer simplification
-      - go build -o /tmp/go-cache-cli-driver ./tests/cli/cache
-      - /tmp/go-cache-cli-driver
-      - rm -f /tmp/go-cache-cli-driver
+      - go run ./tests/cli/cache
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli/cache/Taskfile.yaml` around lines 7 - 9, Replace the fixed /tmp
binary path and brittle cleanup with a unique temporary file and guaranteed
teardown: create a temp path with mktemp (or similar) instead of
/tmp/go-cache-cli-driver, build to that temp path (replace the literal "-o
/tmp/go-cache-cli-driver"), run the binary from that temp path, and ensure
removal happens even if the command fails by registering a trap or finally-style
cleanup that removes the temp binary; update the three commands in the Taskfile
that reference "/tmp/go-cache-cli-driver" accordingly.

Comment thread threats.md
Comment on lines +23 to +26
**Finding:** TBD
**Severity:** TBD
**Repro test:** TBD
**Fix:** TBD
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Replace TBD placeholders with a tracked outcome before release cut.

Lines 23-26 leave the eviction/TOCTOU section unresolved. Please either complete this section now or mark it explicitly out-of-scope with a linked tracking issue.

If helpful, I can draft a concrete Section 3 template (race surfaces, repro matrix, and fix checklist) and a companion issue body.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@threats.md` around lines 23 - 26, Update the eviction/TOCTOU block in
threats.md by replacing the four "TBD" placeholders under
"Finding/Severity/Repro test/Fix" with a tracked outcome: either (A) a completed
entry that identifies the root cause, severity, repro steps, and a concrete
remediation plan, or (B) an explicit "out-of-scope" statement that links to a
tracking issue (create one if missing) and a short rationale; ensure the entry
references "eviction/TOCTOU" and, if applicable, follow the suggested Section 3
template (race surfaces, repro matrix, fix checklist) or point to the linked
issue for that template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant