DeepHide Embedding Engine#2
Merged
Merged
Conversation
add ronomon reed-solomom
…ad to be encrypted
…tream and return the synchronised bitstream
…unpacking the bytes into a Uint8[]
…truction by receiver
…he prepared payload as an arg
…tance and using it in the applyFEC refactor: change functin to be aync
refactor: add bitstream as argument in the recorder function clear logs on injection finish
📝 WalkthroughWalkthroughThis PR implements a complete end-to-end payload embedding pipeline: file loading, framing with metadata, PBKDF2-based key derivation, AES-256-GCM encryption, Reed-Solomon FEC encoding, shard interleaving, bit serialization, and preamble injection, culminating in bitstream feeding to the audio recorder. Changes
Sequence DiagramsequenceDiagram
participant Main as main.ts
participant Gen as preparePayload()
participant FR as FileReader
participant Frame as Framing
participant KD as KeyDerivation
participant Enc as AES Encrypt
participant Pak as Packer
participant FEC as applyFEC
participant IL as Interleave
participant Ser as Serializer
participant Pre as Preamble
participant Rec as recorder()
Main->>Gen: preparePayload("file.txt", "1234")
Gen->>FR: loadFileToUint8("file.txt")
FR-->>Gen: fileBytes
Gen->>Frame: framePayload(fileBytes, "file.txt")
Frame-->>Gen: framedPayload
Gen->>KD: deriveKey("1234", salt)
KD-->>Gen: derivedKey
Gen->>Enc: encryptPayload(framedPayload, key)
Enc-->>Gen: encrypted
Gen->>Pak: packetize(encrypted)
Pak-->>Gen: packets[]
Gen->>FEC: applyFEC(packets, 6, 3)
FEC-->>Gen: shards[]
Gen->>IL: interleave(shards, 6, 3)
IL-->>Gen: interleaved[]
Gen->>Ser: serialiseBits(interleaved)
Ser-->>Gen: bitstream
Gen->>Pre: injectPreamble(bitstream)
Pre-->>Gen: finalBitStream
Gen-->>Main: {finalBitStream, salt}
Main->>Rec: recorder(finalBitStream)
Rec->>Rec: consume bitstream<br/>inject into audio
Rec-->>Main: completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
🧹 Nitpick comments (6)
src/core/embedding/bitstream/preamble.ts (1)
1-12: Minor: JSDoc describes "high-entropy" but preamble starts with alternating pattern.The preamble intentionally starts with 16 low-entropy alternating bits (
1,0,1,0...) for clock recovery, followed by a higher-entropy pattern for frame detection. The comment could be clarified to reflect this two-part design.📝 Suggested documentation improvement
/** * Sync Preamble - * Prepends a high-entropy sync pattern to the bitstream. + * Prepends a sync pattern to the bitstream: alternating bits for clock recovery + * followed by a high-entropy Barker-like sequence for frame detection. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/embedding/bitstream/preamble.ts` around lines 1 - 12, Update the JSDoc for the injectPreamble function and the PREAMBLE constant to describe the two-part design: the initial alternating 16-bit pattern used for clock/bit recovery followed by the subsequent higher-entropy sequence used for frame/sync detection; mention both purposes explicitly and reference PREAMBLE so future readers understand why the array begins with 1,0,1,0... before the high-entropy section.src/core/embedding/crypto/aes.ts (1)
9-11: Consider validating key length.The function assumes
keyis exactly 32 bytes for AES-256, but doesn't validate this. An incorrectly sized key will causecreateCipherivto throw a less helpful error.🛡️ Proposed validation
export function encryptPayload(framedPayload: Uint8Array, key: Uint8Array) { + if (key.length !== 32) { + throw new Error(`Invalid key length: expected 32 bytes, got ${key.length}`); + } const nonce = randomBytes(12); const cipher = createCipheriv("aes-256-gcm", key, nonce);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/embedding/crypto/aes.ts` around lines 9 - 11, encryptPayload currently assumes the provided key is 32 bytes for AES-256 but doesn't validate it, which can cause createCipheriv to throw a cryptic error; add an explicit check in encryptPayload that verifies key.byteLength (or key.length) === 32 and throw a clear, descriptive error (e.g., "Invalid key length: AES-256 requires 32 bytes") before creating nonce or calling createCipheriv, so callers get a helpful message instead of the lower-level exception.src/core/embedding/crypto/keyDerivation.ts (1)
24-26: Consider preserving error context for debugging.The original error is caught but not included in the re-thrown error. This loses valuable debugging information (e.g., was it an invalid salt, memory issue, etc.).
♻️ Proposed fix to preserve error context
} catch (error) { - throw new Error("Failed to derive key"); + throw new Error("Failed to derive key", { cause: error }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/embedding/crypto/keyDerivation.ts` around lines 24 - 26, The catch block in keyDerivation.ts that currently does "throw new Error('Failed to derive key')" discards the original error; update the catch in the deriveKey (or the key derivation) function to preserve context by including the caught error when re-throwing (e.g., pass the original error as the cause or append its message to the new Error) so the original stack/message is retained for debugging.src/core/embedding/fec/readSolomon.ts (2)
43-43: Assumption: All packets must have equal length.This line assumes all packets have the same length as the first packet. This is a valid assumption for RS encoding, but it must be enforced by the caller (
packetize). The variable-length packet issue has been flagged inpacker.ts.Consider adding a defensive check here:
🛡️ Optional: Add length validation
const shardLength = packets[0]!.length; + const invalidPacket = packets.find(p => p.length !== shardLength); + if (invalidPacket) { + throw new Error(`All packets must have equal length. Expected ${shardLength}, found ${invalidPacket.length}`); + } const encodedStream: Uint8Array[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/embedding/fec/readSolomon.ts` at line 43, readSolomon currently assumes all entries in packets are the same length by using packets[0]!.length; add a defensive validation before using shardLength that iterates packets (or uses Array.every) to ensure every packet.length === packets[0]!.length and if not, throw or return an explicit error describing the mismatched packet lengths; reference the packets array and the shardLength variable in readSolomon so the check sits immediately above the const shardLength = ... line and fails fast instead of proceeding with unequal-length packets.
19-20: Use async file operations for WASM loading.
fs.readFileSyncblocks the event loop. SincegetRSEngineis async, usefs.promises.readFilefor consistency. Note thatReedSolomonErasure.fromBytes()is synchronous and should not be awaited.♻️ Async file read
- const wasmBuffer = fs.readFileSync(wasmPath); + const wasmBuffer = await fs.promises.readFile(wasmPath); rsInstance = ReedSolomonErasure.fromBytes(wasmBuffer);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/embedding/fec/readSolomon.ts` around lines 19 - 20, Replace the synchronous read with an async read in getRSEngine: instead of using fs.readFileSync(wasmPath) to produce wasmBuffer, call fs.promises.readFile(wasmPath) and await it, then pass the resulting buffer into ReedSolomonErasure.fromBytes(...) to assign rsInstance; do not await fromBytes since it is synchronous. This keeps getRSEngine non-blocking while preserving the synchronous nature of ReedSolomonErasure.fromBytes.src/main.ts (1)
9-9: Security: Hardcoded password should use environment variable or secure input.The password
"1234"is hardcoded. For production, this should be sourced from a secure location (environment variable, secure prompt, or config).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.ts` at line 9, The call to preparePayload in main.ts currently hardcodes the password "1234"; replace this with a secure source such as an environment variable or secure input (e.g. use process.env.PAYLOAD_PASSWORD or a secret manager/config value) and pass that variable into preparePayload instead of the literal string; update any tests or documentation that assume the hardcoded value and validate that preparePayload receives a non-empty secret before proceeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 26-27: package.json lists dependencies "@noble/ciphers" and
"@noble/hashes" but they are not used in the codebase; either remove these two
entries from dependencies or add a comment/README note and the planned import
locations (e.g., where you intend to use "@noble/ciphers" and "@noble/hashes")
to justify keeping them—update package.json accordingly and run npm/yarn install
to refresh lockfile if you remove them.
In `@src/core/embedding/payload/framing.ts`:
- Around line 20-23: The header writes are overlapping; change the DataView
writes so the magic 0x44484944 is written at offset 0 (view.setUint32(0,...)),
the version/flags byte is written at offset 4 (view.setUint8(4,...)), the
filename length byte stays at offset 5 (view.setUint8(5, filenameBytes.length)),
and the file length is written at offset 6 (view.setUint32(6, filebytes.length,
false)); update the corresponding writes in framing.ts (the lines using
view.setUint32 and view.setUint8) to these offsets and ensure endianness is
consistent for both setUint32 calls.
In `@src/core/embedding/payload/packer.ts`:
- Around line 16-25: The loop in packer.ts creates variable-length frames when
encryptedData.length % frameSize != 0 which breaks applyFEC (it assumes fixed
shardLength); modify the framing logic in the function building frames (use
variables frameSize, encryptedData, frameId, frames) so that every frame payload
is padded to exactly frameSize before being wrapped in the 4-byte header (pad
the last chunk to frameSize with a deterministic value such as zero), and
include the original payload length metadata (either in the 4-byte header
expansion or an extra footer/header packet) so the decoder can strip padding
when unpacking before FEC/decoding.
In `@src/core/embedding/payload/Uint8FileReader.ts`:
- Around line 3-10: The loadFileToUint8 function currently joins user-provided
filename into a path allowing path traversal; validate and constrain filename
before reading: ensure filename contains no directory traversal patterns or path
separators (e.g., reject if it contains "..", "/" or "\"), or better, resolve
the target with path.resolve against path.join(process.cwd(), "data", filename)
and verify the resolved path starts with the resolved data directory path; if
validation fails, throw a descriptive error. Use the function name
loadFileToUint8 and the filepath/readFile usage as the modification points.
In `@src/core/profiler/recorder.ts`:
- Around line 37-40: In recorder.ts inside the loop that checks "if (bitPtr >=
bitstream.length)" (the block that currently logs "SUCCESS! entire bitstream
injected"), restore the original break by uncommenting it so the loop exits once
injection completes; this prevents the recording loop in the function that uses
bitPtr and bitstream from continuing to process audio frames unnecessarily and
wasting CPU/memory.
- Around line 24-31: The loop in recorder.ts reads bits (currentBit) and
advances bitPtr for safebins but never applies those bits to the audio frames
(so transmission is only simulated); update the loop that iterates safebins (and
references bitstream, bitPtr, currentBit) to either (a) actually inject the bit
into the corresponding STFT/bin/frame data used by processSTFT (modify the frame
or frequency bin value where safebins points) so the bit affects the output
audio, or (b) if simulation is intended, make the intent explicit by adding a
TODO comment and returning early or skipping consumption (do not advance bitPtr)
to avoid silently discarding bits; ensure you update or document the code paths
in processSTFT and the loop that consumes safebins/currentBit to reflect the
chosen approach.
In `@src/main.ts`:
- Line 9: preparePayload returns { finalBitStream, salt } but main.ts only
destructures finalBitStream, discarding the 16-byte salt needed for PBKDF2/AES
key derivation; update main.ts to capture salt (const { finalBitStream, salt } =
await preparePayload(...)) and then either prepend the salt to the bitstream
(e.g., combine salt + finalBitStream before sending/storing) or ensure salt is
transmitted/stored alongside finalBitStream so the receiver can reconstruct the
key; reference preparePayload and finalBitStream/salt when making this change.
---
Nitpick comments:
In `@src/core/embedding/bitstream/preamble.ts`:
- Around line 1-12: Update the JSDoc for the injectPreamble function and the
PREAMBLE constant to describe the two-part design: the initial alternating
16-bit pattern used for clock/bit recovery followed by the subsequent
higher-entropy sequence used for frame/sync detection; mention both purposes
explicitly and reference PREAMBLE so future readers understand why the array
begins with 1,0,1,0... before the high-entropy section.
In `@src/core/embedding/crypto/aes.ts`:
- Around line 9-11: encryptPayload currently assumes the provided key is 32
bytes for AES-256 but doesn't validate it, which can cause createCipheriv to
throw a cryptic error; add an explicit check in encryptPayload that verifies
key.byteLength (or key.length) === 32 and throw a clear, descriptive error
(e.g., "Invalid key length: AES-256 requires 32 bytes") before creating nonce or
calling createCipheriv, so callers get a helpful message instead of the
lower-level exception.
In `@src/core/embedding/crypto/keyDerivation.ts`:
- Around line 24-26: The catch block in keyDerivation.ts that currently does
"throw new Error('Failed to derive key')" discards the original error; update
the catch in the deriveKey (or the key derivation) function to preserve context
by including the caught error when re-throwing (e.g., pass the original error as
the cause or append its message to the new Error) so the original stack/message
is retained for debugging.
In `@src/core/embedding/fec/readSolomon.ts`:
- Line 43: readSolomon currently assumes all entries in packets are the same
length by using packets[0]!.length; add a defensive validation before using
shardLength that iterates packets (or uses Array.every) to ensure every
packet.length === packets[0]!.length and if not, throw or return an explicit
error describing the mismatched packet lengths; reference the packets array and
the shardLength variable in readSolomon so the check sits immediately above the
const shardLength = ... line and fails fast instead of proceeding with
unequal-length packets.
- Around line 19-20: Replace the synchronous read with an async read in
getRSEngine: instead of using fs.readFileSync(wasmPath) to produce wasmBuffer,
call fs.promises.readFile(wasmPath) and await it, then pass the resulting buffer
into ReedSolomonErasure.fromBytes(...) to assign rsInstance; do not await
fromBytes since it is synchronous. This keeps getRSEngine non-blocking while
preserving the synchronous nature of ReedSolomonErasure.fromBytes.
In `@src/main.ts`:
- Line 9: The call to preparePayload in main.ts currently hardcodes the password
"1234"; replace this with a secure source such as an environment variable or
secure input (e.g. use process.env.PAYLOAD_PASSWORD or a secret manager/config
value) and pass that variable into preparePayload instead of the literal string;
update any tests or documentation that assume the hardcoded value and validate
that preparePayload receives a non-empty secret before proceeding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71c34920-afbb-4648-a1c5-ccc418ec8313
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.gitignorepackage.jsonsrc/core/embedding/bitstream/preamble.tssrc/core/embedding/bitstream/serialiser.tssrc/core/embedding/crypto/aes.tssrc/core/embedding/crypto/keyDerivation.tssrc/core/embedding/fec/interleave.tssrc/core/embedding/fec/readSolomon.tssrc/core/embedding/generator.tssrc/core/embedding/payload/Uint8FileReader.tssrc/core/embedding/payload/framing.tssrc/core/embedding/payload/packer.tssrc/core/profiler/freqBarkMap.tssrc/core/profiler/recorder.tssrc/main.tstsconfig.json
💤 Files with no reviewable changes (1)
- src/core/profiler/freqBarkMap.ts
Comment on lines
+26
to
+27
| "@noble/ciphers": "^2.1.1", | ||
| "@noble/hashes": "^2.0.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for imports of `@noble` packages in the codebase
rg -n "@noble/ciphers|@noble/hashes" --type tsRepository: Forgata/deephide
Length of output: 42
🏁 Script executed:
# Search for any reference to `@noble` packages in all file types, not just TypeScript
rg "@noble" --type-listRepository: Forgata/deephide
Length of output: 449
🏁 Script executed:
# Search across all file types for `@noble` references
rg "@noble" -iRepository: Forgata/deephide
Length of output: 564
🏁 Script executed:
# Also check the package.json to see the full dependency list and understand context
cat -n package.jsonRepository: Forgata/deephide
Length of output: 1173
Remove unused dependencies or confirm planned use.
@noble/ciphers and @noble/hashes are added to dependencies but are not imported or used anywhere in the codebase. The crypto implementations use Node's built-in node:crypto module instead. Either remove these packages or clarify if they're intended for future use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 26 - 27, package.json lists dependencies
"@noble/ciphers" and "@noble/hashes" but they are not used in the codebase;
either remove these two entries from dependencies or add a comment/README note
and the planned import locations (e.g., where you intend to use "@noble/ciphers"
and "@noble/hashes") to justify keeping them—update package.json accordingly and
run npm/yarn install to refresh lockfile if you remove them.
Comment on lines
+20
to
+23
| view.setUint32(4, 0x44484944, false); | ||
| view.setUint32(4, 0x01); | ||
| view.setUint8(5, filenameBytes.length); | ||
| view.setUint32(6, filebytes.length, false); |
There was a problem hiding this comment.
Critical: Header fields are incorrectly written with overlapping offsets.
The DataView writes have serious issues:
- Line 20 & 21: Both write to offset 4, so line 21 overwrites the magic number
0x44484944with0x01—the identifier is lost. - Offset 0-3: Never written, leaving the first 4 bytes as zeros.
- Offset overlap:
setUint32(6, ...)writes 4 bytes at offsets 6-9, butsetUint8(5, ...)writes 1 byte at offset 5, and the magic should occupy 0-3.
The header layout appears to be intended as:
- Bytes 0-3: Magic identifier
- Byte 4: Version/flags (or use a smaller type)
- Byte 5: Filename length
- Bytes 6-9: File length
🐛 Proposed fix for header layout
- view.setUint32(4, 0x44484944, false);
- view.setUint32(4, 0x01);
- view.setUint8(5, filenameBytes.length);
- view.setUint32(6, filebytes.length, false);
+ view.setUint32(0, 0x44484944, false); // Magic "DHID" at offset 0
+ view.setUint8(4, 0x01); // Version at offset 4
+ view.setUint8(5, filenameBytes.length); // Filename length at offset 5
+ view.setUint32(6, filebytes.length, false); // File length at offset 6📝 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
| view.setUint32(4, 0x44484944, false); | |
| view.setUint32(4, 0x01); | |
| view.setUint8(5, filenameBytes.length); | |
| view.setUint32(6, filebytes.length, false); | |
| view.setUint32(0, 0x44484944, false); // Magic "DHID" at offset 0 | |
| view.setUint8(4, 0x01); // Version at offset 4 | |
| view.setUint8(5, filenameBytes.length); // Filename length at offset 5 | |
| view.setUint32(6, filebytes.length, false); // File length at offset 6 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/embedding/payload/framing.ts` around lines 20 - 23, The header
writes are overlapping; change the DataView writes so the magic 0x44484944 is
written at offset 0 (view.setUint32(0,...)), the version/flags byte is written
at offset 4 (view.setUint8(4,...)), the filename length byte stays at offset 5
(view.setUint8(5, filenameBytes.length)), and the file length is written at
offset 6 (view.setUint32(6, filebytes.length, false)); update the corresponding
writes in framing.ts (the lines using view.setUint32 and view.setUint8) to these
offsets and ensure endianness is consistent for both setUint32 calls.
Comment on lines
+16
to
+25
| while (offset < totalBytes) { | ||
| const end = Math.min(offset + frameSize, totalBytes); | ||
| const chunk = encryptedData.slice(offset, end); | ||
|
|
||
| const frame = new Uint8Array(4 + chunk.length); | ||
| const view = new DataView(frame.buffer); | ||
|
|
||
| view.setUint32(0, frameId, false); | ||
| frame.set(chunk, 4); | ||
| frames.push(frame); |
There was a problem hiding this comment.
Critical: Variable-length packets break downstream FEC encoding.
The last packet will be smaller than frameSize + 4 when encryptedData.length is not divisible by frameSize. However, applyFEC in src/core/embedding/fec/readSolomon.ts (line 43) assumes all packets have the same length as the first packet:
const shardLength = packets[0]!.length;It then allocates fixed-size slots (totalShards * shardLength) and copies each packet assuming it fits exactly. A shorter final packet causes buffer misalignment and corrupted FEC shards.
🐛 Proposed fix: Pad final chunk to frameSize
while (offset < totalBytes) {
const end = Math.min(offset + frameSize, totalBytes);
- const chunk = encryptedData.slice(offset, end);
+ const chunkRaw = encryptedData.slice(offset, end);
+
+ // Pad to frameSize for consistent FEC shard lengths
+ const chunk = new Uint8Array(frameSize);
+ chunk.set(chunkRaw);
const frame = new Uint8Array(4 + chunk.length);Note: The decoder must know the original unpadded length to strip padding. Consider including the original payload length in the framing header or in the last packet.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/embedding/payload/packer.ts` around lines 16 - 25, The loop in
packer.ts creates variable-length frames when encryptedData.length % frameSize
!= 0 which breaks applyFEC (it assumes fixed shardLength); modify the framing
logic in the function building frames (use variables frameSize, encryptedData,
frameId, frames) so that every frame payload is padded to exactly frameSize
before being wrapped in the 4-byte header (pad the last chunk to frameSize with
a deterministic value such as zero), and include the original payload length
metadata (either in the 4-byte header expansion or an extra footer/header
packet) so the decoder can strip padding when unpacking before FEC/decoding.
Comment on lines
+3
to
+10
| export async function loadFileToUint8(filename: string): Promise<Uint8Array> { | ||
| const filepath = path.join(process.cwd(), "data", filename); | ||
| try { | ||
| const buffer = await readFile(filepath); | ||
| return new Uint8Array(buffer.buffer, buffer.byteOffset, buffer.byteLength); | ||
| } catch (error) { | ||
| throw new Error(`Failed to read file: ${filepath}`); | ||
| } |
There was a problem hiding this comment.
Path traversal vulnerability: validate filename input.
The filename parameter is joined directly into the path without sanitization. A malicious filename like "../../etc/passwd" could read arbitrary files outside the data/ directory.
🛡️ Proposed fix to prevent path traversal
import { readFile } from "node:fs/promises";
import path from "node:path";
+
export async function loadFileToUint8(filename: string): Promise<Uint8Array> {
+ // Prevent path traversal by ensuring filename contains no directory components
+ const sanitizedFilename = path.basename(filename);
- const filepath = path.join(process.cwd(), "data", filename);
+ const filepath = path.join(process.cwd(), "data", sanitizedFilename);
try {
const buffer = await readFile(filepath);
return new Uint8Array(buffer.buffer, buffer.byteOffset, buffer.byteLength);
} catch (error) {
- throw new Error(`Failed to read file: ${filepath}`);
+ throw new Error(`Failed to read file: ${sanitizedFilename}`, { cause: error });
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/embedding/payload/Uint8FileReader.ts` around lines 3 - 10, The
loadFileToUint8 function currently joins user-provided filename into a path
allowing path traversal; validate and constrain filename before reading: ensure
filename contains no directory traversal patterns or path separators (e.g.,
reject if it contains "..", "/" or "\"), or better, resolve the target with
path.resolve against path.join(process.cwd(), "data", filename) and verify the
resolved path starts with the resolved data directory path; if validation fails,
throw a descriptive error. Use the function name loadFileToUint8 and the
filepath/readFile usage as the modification points.
Comment on lines
+24
to
+31
| if (safebins.length > 0 && bitPtr < bitstream.length) { | ||
| for (const bitIndex of safebins) { | ||
| if (bitPtr >= bitstream.length) break; | ||
|
|
||
| const currentBit = bitstream[bitPtr]; | ||
|
|
||
| bitPtr++; | ||
| } |
There was a problem hiding this comment.
Bits are consumed but not injected into audio frames.
The code reads currentBit from the bitstream and increments bitPtr, but the bit value is never used to modify the audio signal. The processSTFT function (per context snippet) identifies safe bins but doesn't provide a mechanism to write bits into the audio frames.
This aligns with the PR note that "transmission is currently simulated," but consider adding a TODO comment or returning early to make the simulation intent explicit.
📝 Suggested clarification
for (const bitIndex of safebins) {
if (bitPtr >= bitstream.length) break;
- const currentBit = bitstream[bitPtr];
-
+ // TODO: Implement actual bit injection into audio frame at safeBins[bitIndex]
+ // Currently simulating injection - bits are consumed but not modulated
+ const _currentBit = bitstream[bitPtr];
bitPtr++;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/profiler/recorder.ts` around lines 24 - 31, The loop in recorder.ts
reads bits (currentBit) and advances bitPtr for safebins but never applies those
bits to the audio frames (so transmission is only simulated); update the loop
that iterates safebins (and references bitstream, bitPtr, currentBit) to either
(a) actually inject the bit into the corresponding STFT/bin/frame data used by
processSTFT (modify the frame or frequency bin value where safebins points) so
the bit affects the output audio, or (b) if simulation is intended, make the
intent explicit by adding a TODO comment and returning early or skipping
consumption (do not advance bitPtr) to avoid silently discarding bits; ensure
you update or document the code paths in processSTFT and the loop that consumes
safebins/currentBit to reflect the chosen approach.
Comment on lines
+37
to
+40
| if (bitPtr >= bitstream.length) { | ||
| console.log("SUCCESS! entire bitstream injected"); | ||
| // break; | ||
| } |
There was a problem hiding this comment.
Uncomment break to stop recording after injection completes.
The loop continues indefinitely after logging "SUCCESS" because break is commented out. This wastes CPU/memory by continuing to process audio frames that won't inject any data.
♻️ Proposed fix
if (bitPtr >= bitstream.length) {
console.log("SUCCESS! entire bitstream injected");
- // break;
+ break;
}📝 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
| if (bitPtr >= bitstream.length) { | |
| console.log("SUCCESS! entire bitstream injected"); | |
| // break; | |
| } | |
| if (bitPtr >= bitstream.length) { | |
| console.log("SUCCESS! entire bitstream injected"); | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/profiler/recorder.ts` around lines 37 - 40, In recorder.ts inside
the loop that checks "if (bitPtr >= bitstream.length)" (the block that currently
logs "SUCCESS! entire bitstream injected"), restore the original break by
uncommenting it so the loop exits once injection completes; this prevents the
recording loop in the function that uses bitPtr and bitstream from continuing to
process audio frames unnecessarily and wasting CPU/memory.
| try { | ||
| initialise(); | ||
| console.log("Preparing Bit Stream..."); | ||
| const { finalBitStream } = await preparePayload("file.txt", "1234"); |
There was a problem hiding this comment.
Critical: Salt is discarded, making decryption impossible.
preparePayload returns { finalBitStream, salt }, but only finalBitStream is destructured here. The 16-byte salt is essential for PBKDF2 key derivation during decryption—without it, the receiver cannot reconstruct the AES key.
The salt must be either:
- Prepended to the bitstream (common pattern for encrypted payloads)
- Stored/transmitted separately alongside the bitstream
- Embedded in the framing header before encryption
🔑 Example: Prepend salt to bitstream in generator.ts
// In generator.ts, prepend salt before preamble:
- const finalBitStream = injectPreamble(payloadBits);
- return { finalBitStream, salt };
+ const saltBits = serialiseBits([salt]);
+ const withSalt = new Uint8Array(saltBits.length + payloadBits.length);
+ withSalt.set(saltBits);
+ withSalt.set(payloadBits, saltBits.length);
+ const finalBitStream = injectPreamble(withSalt);
+ return { finalBitStream };Then main.ts can simply use finalBitStream without needing to handle the salt separately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.ts` at line 9, preparePayload returns { finalBitStream, salt } but
main.ts only destructures finalBitStream, discarding the 16-byte salt needed for
PBKDF2/AES key derivation; update main.ts to capture salt (const {
finalBitStream, salt } = await preparePayload(...)) and then either prepend the
salt to the bitstream (e.g., combine salt + finalBitStream before
sending/storing) or ensure salt is transmitted/stored alongside finalBitStream
so the receiver can reconstruct the key; reference preparePayload and
finalBitStream/salt when making this change.
|
Note Docstrings generation - SUCCESS |
coderabbitai Bot
added a commit
that referenced
this pull request
Mar 8, 2026
Docstrings generation was requested by @Forgata. * #2 (comment) The following files were modified: * `src/core/embedding/bitstream/preamble.ts` * `src/core/embedding/bitstream/serialiser.ts` * `src/core/embedding/crypto/aes.ts` * `src/core/embedding/crypto/keyDerivation.ts` * `src/core/embedding/fec/interleave.ts` * `src/core/embedding/fec/readSolomon.ts` * `src/core/embedding/generator.ts` * `src/core/embedding/payload/Uint8FileReader.ts` * `src/core/embedding/payload/framing.ts` * `src/core/embedding/payload/packer.ts` * `src/core/profiler/freqBarkMap.ts` * `src/core/profiler/recorder.ts` * `src/main.ts`
This was referenced Mar 8, 2026
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.
DeepHide Embedding Engine
Implements the DeepHide payload pipeline that converts an input file into a bitstream for injection
Changes
Output
Produces a synchronised, encrypted, FEC-protected bitstream ready for DSSS modulation.
Important
Summary by CodeRabbit
Release Notes
New Features
Chores