-
Notifications
You must be signed in to change notification settings - Fork 0
[feature] SC-166737/improve app proxy security by restricting where token replacements can go #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…oken replacements can go
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnhance app proxy security by refining settingsInjection rules in the manifest and improve type safety in constant definitions via const assertions. Entity relationship diagram for updated settingsInjection in manifest.jsonerDiagram
MindMeister_OAuth_Request {
string url
string[] methods
int timeout
object settingsInjection
}
MeisterTask_API_Request {
string url
string[] methods
int timeout
object settingsInjection
}
MindMeister_OAuth_Request ||--|{ SettingsInjection : contains
SettingsInjection {
string app_id
string client_secret
}
SettingsInjection ||--|{ InjectionTarget : injects
InjectionTarget {
string[] body
}
MeisterTask_API_Request ||--|{ EmptySettingsInjection : contains
EmptySettingsInjection {
}
Class diagram for updated constants with const assertionsclassDiagram
class placeholders {
+ACCESS_TOKEN : string
+CLIENT_ID : string
+CLIENT_SECRET : string
}
class SCOPES {
+["userinfo.email", "userinfo.profile", "meistertask"] : string[]
}
class DESKPRO_LABEL {
+name : string
+color : string
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Ensure that the manifest JSON schema (and any associated TypeScript interfaces) are updated to include the new settingsInjection field so invalid configurations are caught at build time.
- Consider removing the explicit empty settingsInjection object for MeisterTask if downstream code already defaults to no injection, to reduce noise in the manifest.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Ensure that the manifest JSON schema (and any associated TypeScript interfaces) are updated to include the new settingsInjection field so invalid configurations are caught at build time.
- Consider removing the explicit empty settingsInjection object for MeisterTask if downstream code already defaults to no injection, to reduce noise in the manifest.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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request enhances type safety and configuration for OAuth2 token handling in the MeisterTask integration. The changes add stricter TypeScript type inference for constants and configure settings injection for secure credential management in API requests.
- Added
as constassertions to constants for improved type safety and immutability - Configured settings injection for OAuth2 credentials in the proxy whitelist
- Explicitly declared empty settings injection for MeisterTask API endpoints
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/constants.ts | Added as const to placeholders, SCOPES, and DESKPRO_LABEL for stricter type inference |
| manifest.json | Configured settings injection for OAuth2 endpoints and explicitly defined empty injection for MeisterTask API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Build for commit b587fff deployed to: https://meister-task-pr-70.ci.next.deskprodemo.com URLs: |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request introduces improvements to the way configuration and constants are handled for API integrations, focusing on making settings injection and constant definitions more robust and type-safe.
Configuration and settings injection:
manifest.jsonto support dynamic injection ofapp_idandclient_secretinto the body of OAuth requests for MindMeister, improving flexibility and security for credential management.settingsInjectionfor MeisterTask API requests inmanifest.json, clarifying that no settings need to be injected for these endpoints.Type safety improvements:
as constto several constant objects and arrays insrc/constants.ts(placeholders,SCOPES,DESKPRO_LABEL), ensuring stricter type inference and preventing accidental mutation.Summary by Sourcery
Enhance app proxy security by restricting token replacements to specific OAuth request parameters and strengthen type safety on constants
Enhancements: