Skip to content

fix: coerce admin keys to strings in ConfigurationTab (#2353)#2354

Merged
Yeraze merged 1 commit intomainfrom
fix/2353-config-admin-key-trim-crash
Mar 21, 2026
Merged

fix: coerce admin keys to strings in ConfigurationTab (#2353)#2354
Yeraze merged 1 commit intomainfrom
fix/2353-config-admin-key-trim-crash

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Mar 21, 2026

Summary

Fixes #2353 — "ne.trim is not a function" crash on Configuration page.

  • Root cause: The /api/config/current endpoint returns raw protobuf admin keys as serialized Buffer objects ({type:'Buffer', data:[...]}) rather than base64 strings. SecurityConfigSection calls .trim() on these values during render, which crashes because they're objects, not strings.
  • Fix: Convert admin keys to base64 strings when loading from the raw config endpoint in ConfigurationTab.tsx, matching the conversion already done by the formatted /api/device/config/security endpoint.
  • Only affects users who have admin keys configured on their device.

Test plan

  • Configure admin keys on a device, load Configuration tab — should not crash
  • Verify admin keys display correctly as base64 strings
  • Save and reload — keys should persist
  • Full test suite passes (2987 tests)

🤖 Generated with Claude Code

The /api/config/current endpoint returns raw protobuf admin keys as
serialized Buffer objects (e.g., {type:'Buffer', data:[...]}) rather
than base64 strings. SecurityConfigSection calls .trim() on these
during render, causing "ne.trim is not a function" crash.

Convert admin keys to base64 strings when loading from the raw config
endpoint, matching the conversion already done by the formatted
/api/device/config/security endpoint.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 21, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

1 similar comment
@github-actions
Copy link

github-actions bot commented Mar 21, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions
Copy link

github-actions bot commented Mar 21, 2026

Claude finished @Yeraze's task —— View job


PR Review Complete ✅

I've thoroughly analyzed the changes in this PR that fixes the "ne.trim is not a function" crash. Here's my comprehensive review:

Review Checklist:

  • Read and analyze the changed file
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage
  • Provide final review feedback

Overall Assessment: APPROVED

This is a solid, well-targeted bug fix that addresses a real production issue without introducing new risks.

Code Quality & Best Practices ✅

Strengths:

  • Robust type handling: The solution elegantly handles multiple data formats (strings, Buffer objects, byte arrays) that could come from the API
  • Defensive programming: Each case is explicitly handled with proper type checking
  • Clear documentation: Comments explain each transformation case
  • Consistent patterns: Uses the same btoa(String.fromCharCode(...)) pattern found elsewhere in the codebase (ChannelsConfigSection.tsx:393, ChannelDatabaseSection.tsx:428)
  • Minimal scope: Only touches the exact problem area without over-engineering

Bug Analysis ✅

Root cause correctly identified:

  • /api/config/current returns raw protobuf data with Buffer objects: {type:'Buffer', data:[...]}
  • SecurityConfigSection.tsx:172 calls .trim() expecting strings
  • Perfect targeted fix at the data transformation layer

Edge cases handled:

  • Already-string keys (pass-through)
  • JSON-serialized Buffers from protobuf
  • Plain byte arrays
  • Fallback to string coercion for unexpected formats

Performance Considerations ✅

  • Minimal overhead: Only processes admin keys (typically 1-3 items)
  • Efficient operations: String.fromCharCode(...array) + btoa() is optimal for byte-to-base64 conversion
  • No unnecessary work: Early return for string values

Security Review ✅

  • No security risks introduced: Data transformation only, no validation bypassed
  • Proper encoding: Base64 encoding maintains data integrity
  • No sensitive data exposure: Keys remain properly encoded throughout

Test Coverage Assessment ⚠️

Current state:

  • No specific tests found for ConfigurationTab.tsx
  • PR description mentions "Full test suite passes (2987 tests)"
  • The fix is in data transformation logic that would benefit from unit tests

Recommendation: Consider adding a unit test for the admin key transformation logic:

describe('ConfigurationTab admin key handling', () => {
  it('should convert Buffer objects to base64 strings', () => {
    const mockKey = { type: 'Buffer', data: [65, 66, 67] }; // "ABC"
    // Test the conversion logic
  });
});

Minor Suggestions 🔍

  1. Type safety enhancement (src/components/ConfigurationTab.tsx:701):

    // Consider adding explicit typing for better IDE support
    const keys = sec.adminKey.map((key: string | Buffer | {type: 'Buffer', data: number[]} | number[]) => {
  2. Error handling consideration: The current String(key) fallback is good, but could log unexpected formats for debugging.

Comparison with Existing Patterns ✅

The fix aligns perfectly with existing codebase patterns:

  • Same btoa(String.fromCharCode(...)) used in ChannelsConfigSection.tsx and ChannelDatabaseSection.tsx
  • Consistent defensive programming approach
  • Follows the established pattern of handling protobuf data transformations

Final Recommendation: MERGE 🚀

This PR successfully fixes a production crash with a minimal, well-targeted solution. The implementation is robust, follows established patterns, and handles edge cases appropriately. The only minor improvement would be adding unit tests, but the fix itself is solid and ready for production.


@Yeraze Yeraze merged commit 085476d into main Mar 21, 2026
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Configuration failed to load

1 participant