[feature] SC-166737/improve app proxy security by restricting where token replacements can go#61
Conversation
…oken replacements can go
Reviewer's guide (collapsed on small PRs)Reviewer's GuideExtends the Sellsy integration manifest by introducing settingsInjection for automated credential injection in OAuth and API endpoints, and enforces immutability on the placeholders object via a TypeScript const assertion for improved type safety. Entity relationship diagram for updated manifest endpoint configurationerDiagram
SETTINGS_INJECTION {
string client_id
string client_secret
}
ENDPOINT_OAUTH {
string url
string[] methods
int timeout
SETTINGS_INJECTION settingsInjection
}
ENDPOINT_API {
string url
string[] methods
int timeout
SETTINGS_INJECTION settingsInjection
}
SETTINGS_INJECTION ||--|{ ENDPOINT_OAUTH : "used by"
SETTINGS_INJECTION ||--|{ ENDPOINT_API : "used by"
Class diagram for updated placeholders constantclassDiagram
class placeholders {
<<const>>
CLIENT_SECRET : string
ACCESS_TOKEN : string
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:
- Add a brief comment or TODO explaining the purpose of the empty settingsInjection on the API endpoint so its intent is clear to future maintainers.
- Update the TypeScript schema or types for manifest.json to include the new settingsInjection property and catch misconfigurations at compile time.
- Ensure there’s validation or fallback logic in the OAuth flow to handle cases where client_id or client_secret might be missing from settings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a brief comment or TODO explaining the purpose of the empty settingsInjection on the API endpoint so its intent is clear to future maintainers.
- Update the TypeScript schema or types for manifest.json to include the new settingsInjection property and catch misconfigurations at compile time.
- Ensure there’s validation or fallback logic in the OAuth flow to handle cases where client_id or client_secret might be missing from settings.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Build for commit 40bef15 deployed to: https://sellsy-pr-61.ci.next.deskprodemo.com URLs: |
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances security for the Sellsy integration by implementing the settingsInjection feature in the proxy configuration, which restricts where sensitive credentials can be injected in API requests. Additionally, it improves type safety for placeholder constants.
Key changes:
- Added
settingsInjectionconfiguration to the OAuth endpoint to automatically injectclient_idandclient_secretfrom settings into request bodies - Added empty
settingsInjectionobject to the API endpoint for consistency and future extensibility - Applied
as constassertion to theplaceholdersobject for improved type safety
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| manifest.json | Adds settingsInjection configuration to both OAuth and API proxy endpoints to control credential injection |
| src/constants.ts | Marks placeholders object as const with TypeScript assertion for type safety and immutability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request introduces improvements to the Sellsy integration configuration, focusing on better handling of authentication settings and code clarity. The most important changes are grouped below:
Manifest configuration improvements:
settingsInjectionproperty to the OAuth endpoint inmanifest.json, enabling automatic injection ofclient_idandclient_secretfrom settings into the request body. This streamlines authentication setup.settingsInjectionobject to the API endpoint configuration inmanifest.json, preparing for future extensibility and ensuring consistency in endpoint definitions.Codebase clarity:
placeholdersobject insrc/constants.tsasconstusing TypeScript'sas constassertion, improving type safety and preventing accidental mutation.Summary by Sourcery
Introduce settingsInjection configurations in manifest.json to streamline authentication setup for the Sellsy integration and enforce immutability on the placeholders constant for better code safety
New Features:
Enhancements: