Repository navigation
Description
Source-level correctness finding — PV-PUB-GITHUB-CACHE-QUERY-IDENTITY-001
At PR #95's exact head 611b21ee, ConditionalCache.key() sorts parameter names but joins unescaped values with & and =. UrllibTransport.get() instead uses urllib.parse.urlencode(params). GitHubClient.get() uses that raw cache key both to send a conditional ETag and to replay the body after HTTP 304.
Two distinct HTTP requests therefore alias the same cache entry:
from urllib.parse import urlencode
from devostasis.adapters.cache import ConditionalCache
path = "/search/issues"
a = {"q": "state:open&sort=updated"}
b = {"q": "state:open", "sort": "updated"}
assert ConditionalCache.key(path, a) == ConditionalCache.key(path, b)
assert urlencode(a) != urlencode(b)
# Wire A: q=state%3Aopen%26sort%3Dupdated
# Wire B: q=state%3Aopen&sort=updatedAn isolated Python stdlib reproduction confirmed the key collision and distinct encoded URLs. A mocked conditional exchange (A receives HTTP 200 with an ETag; B receives HTTP 304 with the same ETag) demonstrates that B can receive A's cached body. This is a conditional source-level defect: no real GitHub 304 cross-replay, built-in collector collision, production Vital misclassification or installed-package regression run was observed. ETag equality across resources in that scenario is a test assumption, not a provider observation.
Repair / acceptance boundary
- Define one shared request-identity representation for cache and transport. Different transmitted method/origin/path/encoded-query or representation-sensitive headers (Accept/API version) must not share validators. Use consistent parameter encoding/order, or explicitly canonicalize both transport and cache together.
- Isolate configurable API origins and authorization/visibility contexts. Never persist raw credentials or a reversible/unkeyed token fingerprint. If secure scope discrimination is unavailable, isolate caches or cold-invalidate them on context changes.
- Do not reinterpret existing
devostasis.http-cache.v2entries as corrected request identities: bump the cache schema or force a cold miss for incompatible entries. A body digest proves integrity under its key, not request identity. - Add executable RED/GREEN package regressions for (a) colliding A/B requests with separate ETags, (b) mocked A200/B304 equal-ETag cross-replay refusal, (c) legitimate same-request 304 reuse, (d)
&,=, percent escapes, Unicode and parameter ordering, (e) cross-origin/access-context isolation without secret persistence, (f) legacy cache-v2 cold miss, (g) 304 without a valid cached body re-fetch, and (h) cache-on/off evidence and bundle equivalence. Verify at an exact implementation head and run supported CI. - Keep the fix cache-local. No semantic Vital/threshold/window, bundle identity or historical report contract change is justified by this finding.
Deduplication: fix: enforce current cache limits when restoring entries #95 fixes restore-time entry limits, and ConditionalCache exceeds max_entries during a run until save/load #97 covers in-process max_entries growth. Neither covers aliasing of distinct requests. The open/closed cache issues and recent relevant PRs were checked before creating this separate correctness issue.
Publication receipt:DV-RUN547-CACHEKEY-7e18b932.
Activity
andy-zen-dev commentedon Oct 28, 2026
Query-key sub-repair prepared and RED/GREEN tested (partial #98; not release acceptance)
Base: PR #95 head
611b21ee19ad46b4ae6f9ad2dcf872da6dbf7156; recoveredsrc/devostasis/adapters/cache.pyhas independently verified Git blob SHAb6aed04f5e4a2c0b5d5170eda4b742c6872f6faf. The activerelease/0.6.0also still has the unescaped-key defect. No branch/PR/commit/merge was made.A small, partial implementation candidate changes three lines in
cache.py:The actual
UrllibTransport.get()also usesurlencode(params)in insertion order; matching the wire representation (rather than only sorting parameter names) separates{"q":"state:open&sort=updated"}from{"q":"state:open","sort":"updated"}and avoids reinterpreting old unescaped v2 keys. Different wire order now conservatively results in a miss rather than a potentially incorrect hit.Executable implementation-local regression candidate:
tests/test_cache_request_identity.pywith six named cases (DEV-CACHE-QUERY-01..06, ten pytest instances): exact encoded-key match across ASCII/reserved characters/Unicode; distinct-wire key separation; mocked A200/B304 cross-body refusal (equal ETag is a test assumption, not a live provider observation); same-request 304 success; digest-valid saved v2 cold-miss; and wire-order distinction. Also prepared a conformance mapping and CHANGELOG note. Full four-file unified patch SHA-256:55b5ca3008b1d7e1f12b54e992a25b2f3da0c07ccc6238fcfeea77862ec3cde4(7,045 bytes).Verification, bounded to an isolated source-transcribed harness: baseline exact
cache.py(pre-fix) gave 9 failed / 1 passed; query sub-repair gave 10 passed; together with the separately prepared #97 live-cap patch, 19 passed.git apply --check,git apply --whitespace=error, andgit diff --checkpassed in a reconstruction with exact cache blob and synthetic documentation context. This is not a full Devostasis checkout;GitHubClient.get()was the success-path method transcribed from the repository, not the entire installed adapter. Full pytest, wheel, actual upstream CI, origin/access-context isolation and genuine provider exchange are NOT_RUN.Do not close #98 or merge this alone. This sub-repair does not partition a shared cache by API origin, authorization/visibility context, or representation-sensitive request headers. Those are separate correctness/privacy acceptance conditions in the issue and still require an explicit safe design and end-to-end regressions. The v3 schema invalidation is intentionally operational/cache-local and must be coordinated with #95/#97 and independently reviewed before release.
andy-zen-dev commentedon Oct 28, 2026
Additional RED evidence: cache context isolation is still missing (bounded #98 follow-up, 2026-10-28)
Exact code inspected: PR #95
611b21ee19ad46b4ae6f9ad2dcf872da6dbf7156;src/devostasis/adapters/cache.pyblobb6aed04f5e4a2c0b5d5170eda4b742c6872f6faf;src/devostasis/adapters/github.pyblob7a998b94b12547ef543a6320360ed94f088197a7;src/devostasis/cli.py(_cache_for()uses a sharedgithub-etags.json).GitHubClient.get()keys byConditionalCache.key(path, params)only, whileUrllibTransportcan varyapi_baseandtoken(and hardcodes current Accept/API-version headers). Therefore a saved entry for one request authority/credential context can supply the ETag for a different context.Offline reproduction: the real PR #95
cache.pywas recovered and byte-for-byte verified against the Git blob SHA;GitHubClient's success path was source-transcribed, not loaded through a full checkout. Mock transport A returns200+"shared-etag"and an authority-specific body, cache is saved; transport B uses the same URL path/query but a different origin/token/representation. The mock deliberately returns304when it receives the inherited ETag. B then replays A's body (conditional304handling is working as coded, but the validator belonged to another context). Identical ETags across different contexts are a test assumption, not an observed GitHub incident.Four parametrized isolation cases were exercised:
api_base, same token: FAIL (old validator sent);api_base, different tokens: FAIL;The positive control, two identical request contexts with legitimate
304body reuse: PASS.RED results: original PR #95 code 4 failed / 1 passed. Applying the previously posted query-only v3/
urlencodecandidate locally still gives 4 failed / 1 passed. Thus changing query encoding alone definitely does not meet #98 scope isolation. This is an isolated replay with mocked providers, not a proof of cross-account leakage on real GitHub or a production bundle failure.Minimal reproduction pattern for each context pair:
Acceptance/implementation guidance: derive the cache identity from the actual transmitted method, full configured API base/origin, path and encoded query, current representation-affecting headers, and a credential-isolated scope. Do not write raw tokens or reversible/unkeyed token fingerprints. When a custom transport cannot supply a trustworthy cache scope, conservatively bypass conditional caching rather than fall back to path-only identity. Invalidate v2 entries, verify same-context persistent 304 still works, verify origin/token/public-private/header transitions cold-fetch, and check installed-package cache-on/off bundle equivalence. Changing only
ConditionalCache.key()will not suffice becauseGitHubClientmust pass the transport's context into the key.Evidence limits: 5 focused mocked instances on a source-transcribed adapter request path; full package import, native pytest suite, wheel, real external API exchange and GitHub CI for this repair are NOT_RUN. No upstream code change or release acceptance. #98 remains open; this does not block integrating the independently reviewed, narrower restore fix #95.
andy-zen-dev commentedon Oct 28, 2026
Implementation candidate for complete request-context isolation (#98) — 2026-10-28
Pinned base: PR #95 head
611b21ee19ad46b4ae6f9ad2dcf872da6dbf7156, targetingrelease/0.6.0. The source cache module was recovered from the exact Git blobb6aed04f5e4a2c0b5d5170eda4b742c6872f6faf; the relatedgithub.pytransport/client sections and current tests/conformance/changelog were inspected through GitHub. This is a prepared implementation candidate, not a commit or an accepted release change.The six-file candidate includes:
src/devostasis/adapters/cache.py: v2→v3 cold-miss migration; a random 32-byte public per-cache-file salt persisted with the envelope; query identity via the exacturllib.parse.urlencode(params)used on the wire. Malformed/missing salts reject persisted entries rather than guessing.src/devostasis/adapters/github.py:UrllibTransport.cache_scope(scope_salt)derives an opaque SHA-256 scope from GET, the full configured API base (including prefix), Accept and API-version headers, and authorization context. For authenticated requests it uses HMAC-SHA256 with the token as the secret key and the file salt/context as the message; neither raw credentials nor an unkeyed token hash are written. Anonymous and authenticated identities are disjoint.GitHubClient._cache_key()requires an explicit valid scoped identity; custom transports without an identity bypass conditional caching entirely. Scope is recomputed each request, covering token/header changes during one client lifetime.tests/test_cache_request_identity.py: nine implementation-local case IDs, 14 parametrized offline instances covering origin/base prefix/token/anonymous/Accept/API-version changes, legitimate persisted 304, encoded-query collision refusal, v2 cold-miss, unidentified transports, changing tokens, damaged salt, invalid-body 304 fallback, and cache-on/off equivalence for a fresh response.tests/test_rate_limits.py: opt the pre-existing syntheticRecordingTransportinto a stable test-only scope, so existing cross-run 304 and bundle-equivalence tests remain meaningful; update the damaged-body fallback test to seed the scoped key.docs/spec/conformance.mdandCHANGELOG.md: record DEV-CACHE-SCOPE-01..09, the operational v3 migration, and that Vital/policy/bundle semantics are unchanged.Bounded verification: 14/14 offline regression instances PASS on a source-adapted client + urllib Request construction harness using the byte-verified base
cache.py; patchgit apply --check,git apply --whitespace=errorandgit diff --checkPASS against a reconstruction containing exact changed-line contexts. This is not the complete repository checkout, so full installed-package tests, existing conformance suite, historical bundles, wheel, actual GitHub Actions, and GitHub's real HTTP 304 behavior are NOT_RUN. An adversarial fake server returning identical ETags for distinct scopes is a test assumption, not an observed provider incident.Patch artifact SHA-256:
1a584cb5498fa0c548061ca0b9f5b825c20ef947756a1b6926f3f33f6ec95a18, 19,121 bytes, six files; prepared asdevostasis_issue98_complete_scope_candidate.patchin the run artifact. The patch bytes are not committed to GitHub; this comment records their precise implementation/verification boundary, not a claim of publication of the full diff.Next integration gate: merge independently reviewed #95 with write-capable credentials, apply this #98 candidate on a focused branch targeting the release branch, independently review exact diff, run full Python 3.12–3.14 + wheel/historical-conformance CI, and confirm cache-on/off bundle identity. Do not close #98 from these isolated results; do not merge unverified changes or adjust any research semantic rules.
andy-zen-dev commentedon Oct 28, 2026
Follow-up QA on the uncommitted #98 v3-scope candidate — salt admission (2026-10-28)
Scope: review of the prepared six-file patch announced in this issue (SHA-256
1a584cb5498fa0c548061ca0b9f5b825c20ef947756a1b6926f3f33f6ec95a18), pinned to PR #95 head611b21ee19ad46b4ae6f9ad2dcf872da6dbf7156. This defect is in the unmerged candidate, not a claim about merged GitHub code.The proposed
ConditionalCache.load()checkslen(scope_salt) == 64, then callsbytes.fromhex(scope_salt), but never checks the number of decoded bytes. Python'sbytes.fromhexaccepts ASCII whitespace. Consequently a malformed v3 document with"scope_salt": " " * 64is accepted as a zero-byte salt, and"ab" * 30 + " "as a 30-byte salt, contrary to the intended 32-byte per-file salt. The prior DEV-CACHE-SCOPE-07 test covers short/bad-hex/missing strings but misses these.Reproduction on Python:
len(" " * 64) == 64;bytes.fromhex(" " * 64) == b"". Baseline candidate accepts those two malformed inputs; the corrected admission rejects them while keeping a real 64-character/32-byte hex salt.Minimal repair: after the existing
try/except ValueError:Extend
test_dev_cache_scope_07_invalid_or_absent_scope_salt_cold_misseswith" " * 64and"ab" * 30 + " ". No credential, observation, bundle, policy, or semantic rule is involved.Prepared updated candidate (same six files, no new behavior beyond strict salt admission): 19,272 bytes; SHA-256
26a5a22d3f5d606ae8aa64e9cb1e9b0912f3ed40cb270e2b0cd2ff25e74270f6.git apply --check,git apply,git diff --check, and test-module syntax compilation passed against a synthetic worktree reconstructed from the exact prepatch hunk contexts. Focused Python admission oracle: valid hex salt accepted; whitespace-only and padded whitespace strings rejected after correction; malformed hex/short salts remain rejected. Full upstream checkout, pytest, wheel, historical bundles and hosted CI were NOT_RUN.Keep #98 OPEN. Incorporate this small correction into the eventual focused PR after #95 and independently test the complete integrated head. Current GitHub identity
andy-zen-devhaspull=true,push=false,triage=false,maintain=false,admin=false, so this is a code-review/handoff comment, not a commit, PR, merge, or release.andy-zen-dev commentedon Oct 28, 2026
2026-10-28 — independent QA of uncommitted #98 cache-scope candidate:
User-Agentomitted from identityScope / baseline: this finding is against the proposed six-file #98 v2 candidate, not merged Devostasis code. Current dependency PR #95 is still open at
611b21ee19ad46b4ae6f9ad2dcf872da6dbf7156(baserelease/0.6.0). Read actualsrc/devostasis/adapters/github.pyat that head (Git blob7a998b94b12547ef543a6320360ed94f088197a7), the prepared #98 v2 diff and its new test module. No new Issue or PR is warranted.Finding: the proposed
UrllibTransport.cache_scope(salt)includes GET, API base,Accept,X-GitHub-Api-Versionand the authorization context, but omitsself.user_agent, even thoughUrllibTransport.get()sendsrequest.add_header("User-Agent", self.user_agent). Therefore requests made by two otherwise identical transport instances with different user agents receive identical scoped cache keys. If an API/proxy legitimately selects a representation withVary: User-Agent, the proposed cache can reuse the earlier validator/body in the later request context. This is a defensive correctness gap in the candidate, not evidence of an actual GitHub 304 incident.Bounded offline reproduction: mechanically extracted and executed the actual proposed
cache_scope()method from the v2 diff. Two user agents produced equal HMAC scope keys. A syntheticVary: User-AgentHTTP model with an adversarial shared ETag then returned the first agent's body after the second agent received a 304. The amended method produced distinct keys; the second agent issued a cold 200 and received its own body. This model does not claim to reproduce GitHub server behavior or a full package integration.Minimal patch for the uncommitted candidate:
material = (f"GET\\n{self.api_base}\\nAccept:{self.accept}\\n" - f"X-GitHub-Api-Version:{self.api_version}").encode("utf-8") + f"X-GitHub-Api-Version:{self.api_version}\\nUser-Agent:{self.user_agent}").encode("utf-8")Extend the
DEV-CACHE-SCOPE-01parameterized regression with two differinguser_agentvalues. Have its fake transport includeuser_agentin the synthetic response body; assert the second context has noIf-None-Matchand returns the new representation. Document the expanded request-context identity indocs/spec/conformance.mdandCHANGELOG.md.Prepared integration handoff: a complete six-file v3 patch derived from the earlier candidate, SHA-256
4d49873c1a755bce65ae3444ef642462e27d5b29e9afd32a331cad28a9133a4b(saved as an artifact in the current run). This supersedes the uncommitted v2 candidate only; no repository files were modified.Checks actually executed: 10/10 isolated scope/model assertions passed (including old-candidate collision and amended-candidate separation);
git apply --check, synthetic-base apply,git diff --checkand AST parsing of the complete new test module passed. The synthetic base was reconstructed from patch contexts, not a full Devostasis checkout. Full pytest, installed wheel, historical bundle replay, Python 3.12–3.14 CI and integrated HTTP tests remain NOT_RUN. Do not merge #98 until the entire patch is applied to a focused feature branch after #95, these checks pass at the exact new head, and an independent later review accepts it.andy-zen-dev commentedon Oct 28, 2026
#98 candidate QA — same-origin redirects can forward an unrelated ETag (2026-10-28)
Scope / exact source: PR #95 at
611b21ee19ad46b4ae6f9ad2dcf872da6dbf7156,src/devostasis/adapters/github.pyGit blob7a998b94b12547ef543a6320360ed94f088197a7; prepared, uncommitted #98 v3 candidate (User-Agent/salt corrections)._SameOriginRedirects.redirect_request()delegates tourllib.request.HTTPRedirectHandler.redirect_request(), which copiesIf-None-Matchto a redirected request. The proposed v3 cache scope includes origin/base/token/headers and the original path/query, but not the actual final redirect target. Its extra identity fields do not prevent this cross-resource validator forwarding.Reproduction (local loopback HTTP server + real Python 3.13 urllib redirect machinery; no GitHub traffic):
GET /aliasinitially redirects to/v1;/v1returns HTTP 200, ETag"shared", body{"source":"v1"}. Change the alias to redirect to/v2, which has body{"source":"v2"}but deliberately shares that ETag (adversarial test assumption). RepeatGET /aliaswith the cachedIf-None-Match: urllib forwards the old validator to/v2;/v2returns HTTP 304, so the client would replay the old/v1body. Observed request path sequence:/alias(none),/v1(none),/alias("shared"),/v2("shared"). This is a deterministic synthetic correctness counterexample, not evidence that GitHub emitted these ETags or leaked data.Minimal fail-safe repair: keep the existing same-origin authorization/redirect guard but strip the resource-specific conditional validator from the new redirected Request before following 301/302/303/307/308:
Iterate the actual canonicalized header keys:
Request.remove_header("If-None-Match")alone does not remove urllib's stored"If-none-match"key. This fix keeps the original request conditional, but prevents accidental 304 replay at a different destination. It leaves bearer authorization on permitted same-origin redirects intact and continues to reject cross-origin redirects.RED/GREEN evidence: six offline
DEV-CACHE-SCOPE-10test instances using the source-transcribed handler: baseline 5 FAIL / 1 PASS (one check for each redirect code and one cross-origin refusal control); amended handler 6 PASS. Separate real-urllib loopback probe: before fix/v2receives the old validator and returns 304; after fix it receives no conditional header and returns 200 with its actual body. Prepared delta on top of the uncommitted #98 v3 candidate: four files (handler, new test module, conformance mapping, CHANGELOG), 4,033 bytes, SHA-2565a3559320ff569ac6f7204e0f88a04fba7dd558b0b3d04c5c5b860979d0319aa. Synthetic-basegit apply --check/apply/git diff --check: PASS.Limits: tests use a source-transcribed class plus a local standard-library HTTP server; the integrated Devostasis package, full pytest, wheel, historical bundles, and hosted CI are NOT_RUN. No file was committed or PR opened;
andy-zen-devstill haspush=false. Keep #98 open and include redirect safety in the eventual focused PR, then independently verify the exact integrated head.