Skip to content

Conversation

@HappyPaul55
Copy link
Contributor

@HappyPaul55 HappyPaul55 commented Nov 18, 2025

This pull request introduces improvements to configuration and type safety for API integration settings. The main changes are the addition of a new settingsInjection property in manifest.json for injecting sensitive credentials into the request body, and a minor update to enforce type safety in the placeholders object.

Configuration enhancements for credential injection:

  • Added a settingsInjection property to the Sage OAuth API entry in manifest.json, specifying how client_id and client_secret should be injected into the request body.
  • Added an empty settingsInjection object to the Sage Accounting API entry in manifest.json to maintain consistency and future extensibility.

Type safety improvement:

  • Marked the placeholders object in src/constants.ts as const using TypeScript's as const assertion, ensuring stronger type safety for its values.

Summary by Sourcery

Introduce settingsInjection configuration in the app proxy manifest to control credential injection and strengthen placeholder type safety using TypeScript's as const assertion.

New Features:

  • Add settingsInjection property in manifest.json to configure injection of client_id and client_secret into request bodies and include an empty settingsInjection for future extensibility

Enhancements:

  • Enforce const typing on placeholders object in constants.ts for stronger TypeScript type safety

@HappyPaul55 HappyPaul55 requested a review from a team as a code owner November 18, 2025 15:52
@HappyPaul55 HappyPaul55 requested review from Copilot and removed request for a team November 18, 2025 15:53
@sourcery-ai
Copy link

sourcery-ai bot commented Nov 18, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Implements controlled credential injection via a new settingsInjection schema in manifest.json and enforces stronger type safety on placeholders.

Entity relationship diagram for updated manifest.json API configuration

erDiagram
    "Sage OAuth API" {
        string url
        string[] methods
        int timeout
        object settingsInjection
    }
    "Sage Accounting API" {
        string url
        string[] methods
        int timeout
        object settingsInjection
    }
    "settingsInjection" {
        string client_id
        string client_secret
    }
    "Sage OAuth API" ||--o| "settingsInjection" : injects
    "Sage Accounting API" ||--o| "settingsInjection" : injects
    "settingsInjection" {
        string body
    }
Loading

Class diagram for updated placeholders type safety

classDiagram
    class placeholders {
        +string REFRESH_TOKEN
        +string CLIENT_ID
        +string CLIENT_SECRET
    }
    placeholders : <<const>>
Loading

File-Level Changes

Change Details Files
Introduce settingsInjection configuration for secure token placement
  • Configure client_id and client_secret to inject into request bodies for OAuth flows
  • Add empty settingsInjection stub for accounting API to maintain extensibility
manifest.json
Enhance type safety of placeholders object
  • Apply TypeScript ‘as const’ assertion to placeholders to lock literal types
src/constants.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a 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 and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

Copilot AI left a 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 PR improves app proxy security by restricting where sensitive credentials can be injected in API requests. The main focus is on the OAuth token endpoint for Sage integration.

  • Added settingsInjection configuration to the OAuth endpoint in manifest.json to explicitly control where client_id and client_secret are injected
  • Enhanced type safety for the placeholders object in TypeScript using as const assertion

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 assertion to placeholders object for improved type safety
manifest.json Added settingsInjection configuration to OAuth endpoint for controlled credential injection; added empty object to API endpoint for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@openhands-ai
Copy link

openhands-ai bot commented Nov 18, 2025

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • Feature Build

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #74 at branch `SC-166737/improve-app-proxy-security-by-restricting-where-token-replacements-can-go`

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

@github-actions
Copy link

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.

2 participants