[WRONG BRANCH] fix(live): keep frame forensics metadata-only - #180
Draft
luvs01 wants to merge 1 commit into
Draft
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request currently targets @luvs01 Please retarget this PR to Its title has been prefixed with This pull request is being kept as a draft automatically. Once every issue above is resolved, it will be marked ready for review again. |
github-actions
Bot
changed the title
📝 WalkthroughWalkthroughSideband frame diagnostics now record metadata without payload excerpts. They track U+FFFD presence across text and binary payloads. Diagnostic files use owner-only permissions, with tests covering log content and non-Windows permissions. ChangesSideband diagnostic logging
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🤖 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/server/live.ts`:
- Line 116: Update logLiveSidebandFrame in src/server/live.ts to open the frame
log descriptor, enforce mode 0o600 with fchmodSync, append the record through
that descriptor, and close it reliably. In tests/server-live.test.ts:850, retain
the creation-path permission assertion and add a reuse-path case that
pre-creates frameLogPath with mode 0o644, invokes the logger, and verifies it
becomes 0o600.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 18ff4741-3c9f-4f44-bbf0-da4c85da76fc
📒 Files selected for processing (2)
src/server/live.tstests/server-live.test.ts
| fffd, | ||
| }; | ||
| appendFileSync(logPath, `${JSON.stringify(record)}\n`); | ||
| appendFileSync(logPath, `${JSON.stringify(record)}\n`, { mode: 0o600 }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'src/server/live.ts' 'tests/server-live.test.ts' 'package.json' 'bun.lock' 'bun.lockb' 'tsconfig.json'
printf '%s\n' '--- source outline ---'
ast-grep outline src/server/live.ts || true
printf '%s\n' '--- source context ---'
sed -n '1,180p' src/server/live.ts
printf '%s\n' '--- test context ---'
sed -n '810,875p' tests/server-live.test.ts
printf '%s\n' '--- filesystem permission references ---'
rg -n -S 'appendFileSync|fchmodSync|chmodSync|frameLogPath|logPath|forensic|frame log' src tests package.json bun.lock bun.lockb 2>/dev/null || true
printf '%s\n' '--- runtime/config context ---'
for f in package.json bun.lock bun.lockb tsconfig.json; do
if [ -f "$f" ]; then
echo "### $f"
sed -n '1,180p' "$f"
fi
doneRepository: luvs01/opencodex
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- enclosing frame-log test ---'
rg -n -B 45 -A 12 'frameLogPath|OCX_LIVE_FRAME_LOG|frame-log' tests/server-live.test.ts
printf '%s\n' '--- package scripts and runtime declarations ---'
node - <<'JS'
const fs = require("fs");
const pkg = JSON.parse(fs.readFileSync("package.json", "utf8"));
console.log(JSON.stringify({
engines: pkg.engines,
type: pkg.type,
scripts: pkg.scripts,
dependencies: pkg.dependencies,
devDependencies: pkg.devDependencies,
}, null, 2));
JS
printf '%s\n' '--- focused permission patterns ---'
rg -n -S -C 3 'chmodSync\(path, 0o600\)|appendFileSync\(path|openSync\(.*0o600|fchmodSync' src/usage src/server tests/server-live.test.ts
printf '%s\n' '--- standalone existing-file mode probe ---'
node - <<'JS'
"use strict";
const {
appendFileSync,
chmodSync,
mkdtempSync,
readFileSync,
rmSync,
statSync,
} = require("node:fs");
const { join } = require("node:path");
const { tmpdir } = require("node:os");
const dir = mkdtempSync(join(tmpdir(), "ocx-mode-probe-"));
const path = join(dir, "frame.jsonl");
try {
appendFileSync(path, "seed\n", { mode: 0o644 });
chmodSync(path, 0o644);
const before = statSync(path).mode & 0o777;
appendFileSync(path, "record\n", { mode: 0o600 });
const after = statSync(path).mode & 0o777;
console.log(JSON.stringify({ before: before.toString(8), after: after.toString(8), content: readFileSync(path, "utf8") }));
} finally {
rmSync(dir, { recursive: true, force: true });
}
JSRepository: luvs01/opencodex
Length of output: 10915
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun availability ---'
if command -v bun >/dev/null 2>&1; then
bun --version
printf '%s\n' '--- Bun node:fs descriptor probe ---'
bun - <<'JS'
import { chmodSync, closeSync, fchmodSync, mkdtempSync, openSync, readFileSync, rmSync, statSync, writeSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
const dir = mkdtempSync(join(tmpdir(), "ocx-bun-mode-probe-"));
const path = join(dir, "frame.jsonl");
try {
writeSync(openSync(path, "w", 0o644), "seed\n");
chmodSync(path, 0o644);
const before = statSync(path).mode & 0o777;
const fd = openSync(path, "a", 0o600);
fchmodSync(fd, 0o600);
writeSync(fd, "record\n");
closeSync(fd);
const after = statSync(path).mode & 0o777;
console.log(JSON.stringify({ before: before.toString(8), after: after.toString(8), content: readFileSync(path, "utf8") }));
} finally {
rmSync(dir, { recursive: true, force: true });
}
JS
else
echo 'bun unavailable'
fi
printf '%s\n' '--- test imports and TEST_DIR lifecycle ---'
sed -n '1,90p' tests/server-live.test.ts
rg -n -B 8 -A 12 'TEST_DIR|rmSync\(TEST_DIR|mkdirSync\(TEST_DIR|mkdtempSync' tests/server-live.test.tsRepository: luvs01/opencodex
Length of output: 6885
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Bun availability ---'
if command -v bun >/dev/null 2>&1; then
bun --version
printf '%s\n' '--- Bun node:fs descriptor probe ---'
bun - <<'JS'
import { chmodSync, closeSync, fchmodSync, mkdtempSync, openSync, readFileSync, rmSync, statSync, writeSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
const dir = mkdtempSync(join(tmpdir(), "ocx-bun-mode-probe-"));
const path = join(dir, "frame.jsonl");
try {
const seedFd = openSync(path, "w", 0o644);
writeSync(seedFd, "seed\n");
closeSync(seedFd);
chmodSync(path, 0o644);
const before = statSync(path).mode & 0o777;
const fd = openSync(path, "a", 0o600);
fchmodSync(fd, 0o600);
writeSync(fd, "record\n");
closeSync(fd);
const after = statSync(path).mode & 0o777;
console.log(JSON.stringify({ before: before.toString(8), after: after.toString(8), content: readFileSync(path, "utf8") }));
} finally {
rmSync(dir, { recursive: true, force: true });
}
JS
else
echo 'bun unavailable'
fi
printf '%s\n' '--- test imports and TEST_DIR lifecycle ---'
sed -n '1,90p' tests/server-live.test.ts
rg -n -B 8 -A 12 'TEST_DIR|rmSync\(TEST_DIR|mkdirSync\(TEST_DIR|mkdtempSync' tests/server-live.test.tsRepository: luvs01/opencodex
Length of output: 6885
Harden reused frame log files before appending.
{ mode: 0o600 } affects only file creation. If OCX_LIVE_FRAME_LOG points to an existing 0o644 file, logLiveSidebandFrame keeps its permissive mode. On POSIX, open the file, call fchmodSync(fd, 0o600), append the record, and close the descriptor.
In tests/server-live.test.ts, retain the creation-path assertion and add a reuse-path case that pre-creates frameLogPath with 0o644, invokes the logger, and asserts 0o600.
📍 Affects 2 files
src/server/live.ts#L116-L116(this comment)tests/server-live.test.ts#L850-L850
🤖 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/server/live.ts` at line 116, Update logLiveSidebandFrame in
src/server/live.ts to open the frame log descriptor, enforce mode 0o600 with
fchmodSync, append the record through that descriptor, and close it reliably. In
tests/server-live.test.ts:850, retain the creation-path permission assertion and
add a reuse-path case that pre-creates frameLogPath with mode 0o644, invokes the
logger, and verifies it becomes 0o600.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
contexttext from frames and record only metadata fields:ts,dir,kind,bytes, and a booleanfffdflag when U+FFFD is present in decoded text, implemented inlogLiveSidebandFrameinsrc/server/live.ts.String.prototype.includesover the decoder result and do not persist surrounding text.0o600) by passing{ mode: 0o600 }toappendFileSyncso new files are private on POSIX systems.tests/server-live.test.tsto assert that nocontextproperty or payload text is present in the log and to check POSIX file mode is0600when applicable.Testing
bun run typecheck, which succeeded (no type errors).bun run test -- tests/server-live.test.ts --test-name-pattern "sideband frame log records metadata without payload content", which passed and verifies metadata-only logging and file permissions.bun run privacy:scan, which passed.bun run testwas started under the pinned Bun runtime; it exercised many suites but an unrelated existing test (tests/server-auth.test.ts) failed (expected 401 vs received 403) during the full run and that unrelated failure was observed while the overall long suite was running.Codex Task
Summary by CodeRabbit