[feature] SC-166737/improve app proxy security by restricting where token replacements can go#65
Conversation
…oken replacements can go
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduce a new settingsInjection section in manifest.json to explicitly map authentication credentials into specific request body fields for enhanced security, and enforce read-only type safety on the placeholders object by adding 'as const'. Class diagram for updated placeholders constantclassDiagram
class placeholders {
<<const>>
ACCESS_TOKEN : string
GLOBAL_ACCESS_TOKEN : string
GLOBAL_REFRESH_TOKEN : string
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Update your manifest.json schema/TypeScript interfaces to include the new settingsInjection field so you get compile-time validation of your injection mappings.
- Add a runtime validation step to ensure settingsInjection keys align with actual placeholders and prevent misconfiguration or accidental token injection into unintended fields.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Update your manifest.json schema/TypeScript interfaces to include the new settingsInjection field so you get compile-time validation of your injection mappings.
- Add a runtime validation step to ensure settingsInjection keys align with actual placeholders and prevent misconfiguration or accidental token injection into unintended fields.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances security and type safety in the Lansweeper Deskpro app by restricting where authentication credentials can be injected and improving TypeScript type definitions.
- Adds a
settingsInjectionconfiguration to the API proxy whitelist entry, explicitly defining which settings can be injected into specific request body fields - Applies
as constassertion to theplaceholdersobject for improved type safety and immutability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/constants.ts |
Adds as const assertion to the placeholders object to make it immutable and provide better type inference |
manifest.json |
Introduces settingsInjection configuration that maps authentication settings (client_id, client_secret, global_access_token) to specific request body fields, restricting where credential replacements can occur in proxied API requests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Build for commit c2c56dc deployed to: https://lansweeper-pr-65.ci.next.deskprodemo.com URLs: |
This pull request introduces improvements to how authentication credentials are injected into API requests and adds type safety to the
placeholdersobject in the codebase. The most important changes are grouped below:Authentication Settings Injection:
manifest.jsonAPI configuration to include a newsettingsInjectionobject, which mapsclient_id,client_secret, andglobal_access_tokento specific fields in the request body. This change allows for more flexible and secure injection of authentication credentials into API calls.Type Safety Enhancements:
as constto theplaceholdersobject insrc/constants.ts, ensuring that the object is treated as a read-only constant and improving type safety throughout the codebase.Summary by Sourcery
Introduce secure injection of authentication credentials in API requests and enhance type safety for placeholder constants
New Features:
Enhancements: