Skip to content

fix(channels): leftovers from #153 — stale lock-text write, '|' in cache keys, ignore corescope-ingestor #163

Description

@dborup

Summary

Three small leftovers found while fixing #152 in PR #153 (round 4, head dde4fc98). The PR left them alone because they were out of scope. None blocks #153.

Relates to #152, #153.

1. The "no decryption key" lock text can overwrite a newer request's view (P3)

Where: public/channels.js ~2530–2543 at dde4fc98 (selectChannel, the encrypted-channel branch). The same code is on master 727efca0 at ~2365.

Problem: for an encrypted channel, selectChannel walks the stored keys and awaits ChannelDecrypt.computeChannelHash once per key. When no key matches, it writes the #781 lock message ("This channel is encrypted and no decryption key is configured"). It does not check isStaleMessageRequest(request) before that write.

If the user selects another channel, or the region changes, while that loop is still awaiting, the lock text can land on top of the newer request's pane. This is the same late-write pattern that #153 fixed for "Decrypting messages…".

Fix: check isStaleMessageRequest(request) after the loop, and after each await where it matters, before writing to msgEl. Apply the same check to the other lock-text writes in that function if they follow an await.

Test: a unit test in the style of the R4-3 tests in test-channels-client-state-152.js:

  1. Select encrypted channel A with several stored keys and slow computeChannelHash.
  2. Select B before the loop finishes.
  3. B's view must not be overwritten by the lock text.

The test must fail on the current code.

2. A | in a channel name can clear another channel's cache (theoretical)

Where: public/channel-decrypt.js: CACHE_REGION_SEP = '|' (~361), channelCacheKey (~363) and clearChannelCache (~504).

Problem: cache keys are <channel>|<sorted regions>, and removing a key clears <channel> plus every key that starts with <channel>|. Take two channels named #a and #a|b: removing #a also clears #a|b's cache, and #a|b's entries can be confused with a region-scoped entry of #a.

Today this costs at most a re-decrypt, and it does not leak plaintext. It is still worth making the key unambiguous.

Fix: use a separator that cannot appear in a channel name, or encode the channel part of the key, for example with encodeURIComponent. The one-time migration that #153 added can rewrite any existing keys.

Test: channels #a and #a|b, each with cached entries. Removing #a leaves #a|b's entries intact.

3. corescope-ingestor is not in .gitignore

Where: .gitignore lists corescope-server (~33) but not corescope-ingestor.

Problem: every local E2E build of the ingestor binary leaves an untracked file in the repo root, so agents have to delete it by hand before committing. That invites an accidental commit.

Fix: add corescope-ingestor next to corescope-server.

Acceptance criteria

  • Item 1: a regression test that fails before the fix and passes after it.
  • Item 2: a unit test as described above. Existing cache and migration tests stay green.
  • Item 3: git status is clean after go build -o corescope-ingestor ./cmd/ingestor in the repo root.
  • deploy.yml and its fork guards are unchanged.

No activity

Activity on this issue will appear here.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions