Skip to content

Comments

feat: implement full radius attribute parsing and generic vsa support#2

Merged
essinghigh merged 1 commit intomainfrom
radius-attribute-parsing-16644263998051697360
Feb 18, 2026
Merged

feat: implement full radius attribute parsing and generic vsa support#2
essinghigh merged 1 commit intomainfrom
radius-attribute-parsing-16644263998051697360

Conversation

@essinghigh
Copy link
Member

Implemented full RADIUS attribute parsing to support all standard attributes (RFC 2865, 2866, etc.) and generic Vendor-Specific Attributes (VSAs).

  • Added src/dictionary.ts containing a comprehensive list of standard RADIUS attributes and their types.
  • Created src/helpers.ts with decoding functions for various data types (string, integer, ipaddr, ipv6addr, date, ifid, etc.) and a generic VSA parser.
  • Refactored src/protocol.ts to iterate through all attributes in the response packet and decode them into a structured attributes array in RadiusResult.
  • Updated RadiusResult type definition to include attributes.
  • Ensured backward compatibility by preserving the existing class field extraction logic.
  • Added unit tests in tests/helpers.test.ts to verify attribute decoding logic.
  • Updated Access-Reject and Access-Challenge handling to parse attributes as well.

PR created automatically by Jules for task 16644263998051697360 started by @essinghigh

@google-labs-jules
Copy link

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@opencode-agent
Copy link

Code Review Summary

Issues Found

1. Type Safety Concerns

  • src/helpers.ts:129: Uses any type for decodedValue, losing type safety
  • src/types.ts:34: VendorSpecificAttribute.value uses any, making the API weakly typed

2. Potential Bugs

  • src/helpers.ts:108: VSA parsing returns raw hex string when subAttributes.length === 0 even if parsable === true:

    value: parsable && subAttributes.length > 0 ? subAttributes : data.toString("hex")

    Should check subAttributes.length > 0 differently or handle empty array case.

  • src/helpers.ts:38-52: decodeIpv6Prefix doesn't validate that prefixBuffer is at most 16 bytes before copying to fullAddress

  • src/helpers.ts:24-27: decodeIpAddr returns "0.0.0.0" for invalid buffer length, which is a valid IP - should return null or use a sentinel value

3. Incorrect Type Definitions

  • src/dictionary.ts:24: Type 24 (State) should be string (it's opaque per RFC 2865), but this is actually correct as opaque bytes are typically represented as strings in implementations

4. IPv6 Address Formatting (RFC 5952)

  • src/helpers.ts:29-36: decodeIpv6Addr doesn't compress consecutive zero segments (e.g., returns 2001:db8:0:0:0:0:0:1 instead of 2001:db8::1)

5. Edge Case Handling

  • src/helpers.ts:9,14,19,25,30: Decoders return 0, BigInt(0), new Date(0), "0.0.0.0", "::" for invalid buffers respectively - these are ambiguous values (could be real data or error indicators)

6. Missing Tests

  • No test coverage for malformed packets in protocol.ts
  • No test for decodeIfId

7. Code Duplication

  • src/protocol.ts:149-196: The target attribute extraction logic is somewhat convoluted with nested conditions. Consider extracting into a separate function.

Minor Suggestions

  • Consider adding JSDoc comments to exported functions
  • The dictionary comment on line 224 says "// ... and so on" but has actual content - confusing
  • Could add a vendorId field to regular ParsedAttribute for consistency when VSA is parsed

Overall

The implementation is solid and handles the core use cases well. The main concerns are the type safety issues and some edge cases in the decoder functions that could cause subtle bugs. The VSA parsing is correctly implemented with generic sub-attribute handling.

New%20session%20-%202026-02-18T20%3A07%3A44.229Z
opencode session  |  github run

@opencode-agent
Copy link

User opencode-agent[bot] does not have write permissions

github run

@essinghigh essinghigh merged commit 715eed5 into main Feb 18, 2026
2 checks passed
@essinghigh essinghigh deleted the radius-attribute-parsing-16644263998051697360 branch February 19, 2026 10:37
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.

1 participant