fix(OSQUERY-001): 21 review findings across 7 files

[flamingo]

Closes 21 review findings across 7 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

#	Fix confidence	Finding	Location
1	🟢 95 high	openframe_encryption_service.cpp missing required copyright/SPDX header	openframe/openframe_encryption_service.cpp:1
2	🟡 85 medium	OpenframeEncryptionService defined outside the osquery namespace	openframe/openframe_encryption_service.cpp:8
3	🟡 70 medium	OpenframeEncryptionService throws std::runtime_error instead of returning Status	openframe/openframe_encryption_service.cpp:17
4	🟡 75 medium	openframe_encryption_service.cpp violates all three openframe/ structural conventions	openframe/openframe_encryption_service.cpp:1
5	🟡 80 medium	AES-256-GCM decrypt uses raw secret string as key without deriving/validating 32-byte key length	openframe/openframe_encryption_service.cpp:36
6	🟡 85 medium	handleOpenSSLError builds diagnostic string without using LOG() macros	openframe/openframe_encryption_service.cpp:88
7	🟢 95 high	openframe_token_extractor.cpp missing the standard osquery copyright/SPDX header	openframe/openframe_token_extractor.cpp:1
8	🟢 90 high	OpenframeTokenExtractor class defined outside the osquery namespace	openframe/openframe_token_extractor.cpp:5
9	🟡 60 medium	OpenframeTokenExtractor throws std::runtime_error instead of returning osquery::Status	openframe/openframe_token_extractor.cpp:20
10	🟡 70 medium	openframe_token_extractor.cpp has no diagnostic logging via LOG()/VLOG()	openframe/openframe_token_extractor.cpp:33
11	🟢 95 high	Missing osquery copyright/SPDX header in openframe_token_refresher.cpp	openframe/openframe_token_refresher.cpp:1
12	🟡 70 medium	OpenframeTokenRefresher constructor throws std::runtime_error instead of returning Status	openframe/openframe_token_refresher.cpp:6
13	🟡 75 medium	openframe_token_refresher.cpp violates all three openframe/ structural conventions simultaneously	openframe/openframe_token_refresher.cpp:1
14	🟢 92 high	openframe_encryption_service.h missing standard osquery copyright/SPDX header	openframe/openframe_encryption_service.h:1
15	🟡 88 medium	OpenframeEncryptionService class defined outside the osquery namespace	openframe/openframe_encryption_service.h:12
16	🟡 70 medium	decrypt() documented to throw std::runtime_error instead of returning Status	openframe/openframe_encryption_service.h:14
17	🟢 96 high	openframe_token_extractor.h missing standard osquery copyright/SPDX header	openframe/openframe_token_extractor.h:1
18	🟢 90 high	OpenframeTokenExtractor class defined outside the osquery namespace	openframe/openframe_token_extractor.h:1
19	🔴 35 low — review closely	openframe include headers referenced in init.cpp lack the standard osquery copyright/SPDX header	osquery/core/init.cpp:98
20	🔴 55 low — review closely	initOpenFrame() uses try/catch and std::exception instead of osquery's Status-based error handling	osquery/core/init.cpp:213
21	🟢 90 high	openframe_token_refresher.h missing standard osquery copyright/SPDX header	openframe/openframe_token_refresher.h:1

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

---

Run: https://product-hub.flamingo.so/admin/code-review
Run id: 01e7aadc-0204-47ef-b373-fc5dbadf3ab5

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

[flamingo]

🦩 What this fix changed, finding by finding

21 finding(s) fixed in this draft — 21 explained inline on the diff; 2 low-confidence hunk(s) need close review before merging.

[flamingo]

🦩 🔴 openframe_encryption_service.cpp missing required copyright/SPDX header

Added the canonical osquery copyright/SPDX header block at the top of openframe/openframe_encryption_service.cpp, before the #include block, exactly as suggested in the finding.

🤖 Prompt for AI agents

In openframe/openframe_encryption_service.cpp around line 1, review and complete this code-review fix: openframe_encryption_service.cpp missing required copyright/SPDX header.
What the draft fix changed: Added the canonical osquery copyright/SPDX header block at the top of openframe/openframe_encryption_service.cpp, before the #include block, exactly as suggested in the finding.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 OpenframeEncryptionService defined outside the osquery namespace

Wrapped all definitions (constructor, decrypt, base64Decode, handleOpenSSLError) in namespace osquery { ... } at the end of the file. This requires the corresponding header file to also declare the class inside namespace osquery, which is outside this file's scope — flagged as a dependency risk.

🤖 Prompt for AI agents

In openframe/openframe_encryption_service.cpp around line 8, review and complete this code-review fix: OpenframeEncryptionService defined outside the osquery namespace.
What the draft fix changed: Wrapped all definitions (constructor, decrypt, base64Decode, handleOpenSSLError) in `namespace osquery { ... }` at the end of the file. This requires the corresponding header file to also declare the class inside `namespace osquery`, which is outside this file's scope — flagged as a dependency risk.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 OpenframeEncryptionService throws std::runtime_error instead of returning Status

Changed decrypt() and the constructor to return osquery::Status instead of throwing std::runtime_error; decrypt() now takes an out-parameter std::string& result and returns Status, matching the aws_firehose.cpp convention. This is a signature/API change that requires updating the header (openframe_encryption_service.h) and all call sites, which are not visible in this file — those changes are necessary for the code to compile and are unverified here.

🤖 Prompt for AI agents

In openframe/openframe_encryption_service.cpp around line 17, review and complete this code-review fix: OpenframeEncryptionService throws std::runtime_error instead of returning Status.
What the draft fix changed: Changed decrypt() and the constructor to return osquery::Status instead of throwing std::runtime_error; decrypt() now takes an out-parameter `std::string& result` and returns Status, matching the aws_firehose.cpp convention. This is a signature/API change that requires updating the header (openframe_encryption_service.h) and all call sites, which are not visible in this file — those changes are necessary for the code to compile and are unverified here.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 openframe_encryption_service.cpp violates all three openframe/ structural conventions

Same structural fix as above: header, namespace, and Status-based error handling applied throughout the file (constructor no longer throws, decrypt/base64Decode/handleOpenSSLError all return Status). Full conformance depends on updating the header file declarations accordingly.

🤖 Prompt for AI agents

In openframe/openframe_encryption_service.cpp around line 1, review and complete this code-review fix: openframe_encryption_service.cpp violates all three openframe/ structural conventions.
What the draft fix changed: Same structural fix as above: header, namespace, and Status-based error handling applied throughout the file (constructor no longer throws, decrypt/base64Decode/handleOpenSSLError all return Status). Full conformance depends on updating the header file declarations accordingly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 75 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 AES-256-GCM decrypt uses raw secret string as key without deriving/validating 32-byte key length

Added an explicit length check if (secret_.size() != 32) in decrypt() before calling EVP_DecryptInit_ex, returning Status::failure() if the secret is not exactly 32 bytes, preventing the out-of-bounds/truncation issue with the raw secret used as an AES-256 key. Does not add a proper KDF (e.g. HKDF) — a complete fix would derive the key rather than only validating length, which is a larger design decision left for review.

🤖 Prompt for AI agents

In openframe/openframe_encryption_service.cpp around line 36, review and complete this code-review fix: AES-256-GCM decrypt uses raw secret string as key without deriving/validating 32-byte key length.
What the draft fix changed: Added an explicit length check `if (secret_.size() != 32)` in decrypt() before calling EVP_DecryptInit_ex, returning Status::failure() if the secret is not exactly 32 bytes, preventing the out-of-bounds/truncation issue with the raw secret used as an AES-256 key. Does not add a proper KDF (e.g. HKDF) — a complete fix would derive the key rather than only validating length, which is a larger design decision left for review.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 80 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 openframe_token_extractor.h missing standard osquery copyright/SPDX header

Added the canonical osquery copyright/SPDX header block at the top of the file, before the #pragma once line, matching the format used in other production headers.

🤖 Prompt for AI agents

In openframe/openframe_token_extractor.h around line 1, review and complete this code-review fix: openframe_token_extractor.h missing standard osquery copyright/SPDX header.
What the draft fix changed: Added the canonical osquery copyright/SPDX header block at the top of the file, before the `#pragma once` line, matching the format used in other production headers.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 96 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 OpenframeTokenExtractor class defined outside the osquery namespace

Wrapped the OpenframeTokenExtractor class declaration in namespace osquery { ... }, opening the namespace after the includes and closing it with } // namespace osquery at the end of the file, consistent with the sibling header openframe_authorization_manager.h.

🤖 Prompt for AI agents

In openframe/openframe_token_extractor.h around line 1, review and complete this code-review fix: OpenframeTokenExtractor class defined outside the osquery namespace.
What the draft fix changed: Wrapped the `OpenframeTokenExtractor` class declaration in `namespace osquery { ... }`, opening the namespace after the includes and closing it with `} // namespace osquery` at the end of the file, consistent with the sibling header `openframe_authorization_manager.h`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 openframe include headers referenced in init.cpp lack the standard osquery copyright/SPDX header

Added a comment block above the #include "openframe/..." lines in init.cpp flagging that those headers are missing the canonical osquery copyright/SPDX header per OSQUERY-001, and that they must be fixed before further merges. This is the only in-file remediation possible since the actual header text must be added to files (openframe_.h/.cpp) not shown/editable here; a complete fix requires prepending the standard license header block to each openframe/.h and openframe/*.cpp file, which is out of scope for this single-file change.

🤖 Prompt for AI agents

In osquery/core/init.cpp around line 98, review and complete this code-review fix: openframe include headers referenced in init.cpp lack the standard osquery copyright/SPDX header.
What the draft fix changed: Added a comment block above the `#include "openframe/..."` lines in `init.cpp` flagging that those headers are missing the canonical osquery copyright/SPDX header per OSQUERY-001, and that they must be fixed before further merges. This is the only in-file remediation possible since the actual header text must be added to files (openframe_*.h/.cpp) not shown/editable here; a complete fix requires prepending the standard license header block to each openframe/*.h and openframe/*.cpp file, which is out of scope for this single-file change.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 35 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 initOpenFrame() uses try/catch and std::exception instead of osquery's Status-based error handling

Refactored initOpenFrame() in osquery/core/init.cpp to remove the try { ... } catch (const std::exception& e) block and instead call token_refresher->start() and check its return via .ok()/.getMessage(), matching the Status-based pattern used elsewhere (e.g. initWorkDirectories). This assumes OpenframeTokenRefresher::start() can be changed to return osquery::Status instead of void; that signature change lives in openframe_token_refresher.h/.cpp, which are not visible/editable here, so this file alone will not compile until that companion change is made. If start() cannot be changed to return Status, this edit needs revising (e.g., keep start() void and only remove the try/catch around synchronous calls that already don't throw), which is a risk a reviewer must resolve by checking the actual OpenframeTokenRefresher interface.

🤖 Prompt for AI agents

In osquery/core/init.cpp around line 213, review and complete this code-review fix: initOpenFrame() uses try/catch and std::exception instead of osquery's Status-based error handling.
What the draft fix changed: Refactored `initOpenFrame()` in `osquery/core/init.cpp` to remove the `try { ... } catch (const std::exception& e)` block and instead call `token_refresher->start()` and check its return via `.ok()`/`.getMessage()`, matching the Status-based pattern used elsewhere (e.g. `initWorkDirectories`). This assumes `OpenframeTokenRefresher::start()` can be changed to return `osquery::Status` instead of `void`; that signature change lives in `openframe_token_refresher.h/.cpp`, which are not visible/editable here, so this file alone will not compile until that companion change is made. If `start()` cannot be changed to return Status, this edit needs revising (e.g., keep `start()` void and only remove the try/catch around synchronous calls that already don't throw), which is a risk a reviewer must resolve by checking the actual OpenframeTokenRefresher interface.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 openframe_token_refresher.h missing standard osquery copyright/SPDX header

Prepended the standard osquery Copyright/SPDX header block (matching the format used in other compliant headers like aws_kinesis.h) immediately before #pragma once at the top of openframe/openframe_token_refresher.h. No other lines were changed.

🤖 Prompt for AI agents

In openframe/openframe_token_refresher.h around line 1, review and complete this code-review fix: openframe_token_refresher.h missing standard osquery copyright/SPDX header.
What the draft fix changed: Prepended the standard osquery Copyright/SPDX header block (matching the format used in other compliant headers like aws_kinesis.h) immediately before `#pragma once` at the top of openframe/openframe_token_refresher.h. No other lines were changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer