Skip to content

Checking int fields for params and limiting retrieval amount #1089

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

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

ArneTR
Copy link
Member

@ArneTR ArneTR commented Mar 8, 2025

Greptile Summary

Added integer field validation and retrieval limits to the API endpoints to enhance security and prevent excessive data retrieval.

  • Added check_int_field_api() in api/api_helpers.py to validate integer parameters with type, minimum and maximum value checks
  • Implemented validation for machine_id parameter (max 1024) across API endpoints in api/main.py
  • Added required limit parameter with default=5 and max=50 for /v1/runs endpoint
  • Added validation before using integer parameters in database queries to prevent SQL injection and overflow attacks

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -192,6 +193,8 @@ def get_timeline_query(user, uri, filename, machine_id, branch, metrics, phase,
if branch is None or branch.strip() == '':
branch = 'main'

check_int_field_api(machine_id, 'machine_id', 1024) # can cause exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The comment 'can cause exception' is redundant since the function name and its usage clearly indicate exception handling

Copy link

github-actions bot commented Mar 8, 2025

Eco-CI Output:

Label 🖥 avg. CPU utilization [%] 🔋 Total Energy [Joules] 🔌 avg. Power [Watts] Duration [Seconds]
Total Run (incl. overhead) 28.0723 2329.81 4.08 570.98
Measurement #1 28.0398 2329.81 4.09 569.10

🌳 CO2 Data:
City: Washington, Lat: 38.7095, Lon: -78.1539
IP: 74.235.134.177
CO₂ from energy is: 0.920274950 g
CO₂ from manufacturing (embodied carbon) is: 0.162908478 g
Carbon Intensity for this location: 395 gCO₂eq/kWh
SCI: 1.083183 gCO₂eq / pipeline run emitted


Total cost of whole PR so far:

@ArneTR ArneTR merged commit 156ece8 into main Mar 8, 2025
1 check passed
@ArneTR ArneTR deleted the api-retrieval-limit branch March 8, 2025 11:37
ArneTR added a commit that referenced this pull request Mar 14, 2025
* main: (54 commits)
  Optimizations can get data without HTTP (#1090)
  Checking int fields for params and limiting retrieval amount (#1089)
  E-Mail now made optional
  Bump deepdiff from 8.2.0 to 8.3.0 (#1087)
  Bump psycopg-pool from 3.2.5 to 3.2.6 (#1081)
  Bump pytest from 8.3.4 to 8.3.5 (#1084)
  Bump fastapi[standard] from 0.115.8 to 0.115.11 (#1083)
  Update README.md - projects -> products
  Update README.md
  Updated gcb_playwright to 1.50.0 Will be build with 1.50.1 PR
  Bump psycopg-pool from 3.2.4 to 3.2.5 (#1078)
  Bump playwright/python in /docker/auxiliary-containers/gcb_playwright (#1072)
  Bump aiohttp from 3.11.12 to 3.11.13 (#1080)
  Bump psycopg[binary] from 3.2.4 to 3.2.5 (#1077)
  Bump cachetools from 5.5.1 to 5.5.2 (#1076)
  Updated Cloud Energy
  Typo
  Eco-CI now has a carbon badge in the repo overview [skip ci] (#1079)
  Updated Cloud Energy
  No asking for send ping in Codespaces; When asked though never send ping automatically
  ...
davidkopp pushed a commit to davidkopp/green-metrics-tool that referenced this pull request Mar 14, 2025
ArneTR added a commit that referenced this pull request Mar 16, 2025
* Add support for entrypoint attribute

* Fix entrypoint implementation

* Improve test case

* Add test case with script as entrypoint

* Add test case for entrypoint in conjunction with command

* Bump deepdiff from 8.2.0 to 8.3.0 (#1087)

Bumps [deepdiff](https://github.com/seperman/deepdiff) from 8.2.0 to 8.3.0.
- [Release notes](https://github.com/seperman/deepdiff/releases)
- [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst)
- [Commits](seperman/deepdiff@8.2.0...8.3.0)

---
updated-dependencies:
- dependency-name: deepdiff
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* E-Mail now made optional

* Checking int fields for params and limiting retrieval amount (#1089)

* Add compose stack name + allow test stack to run in parallel (#1093)

* Add compose stack name

* Allow test stack to run in parallel

* Fix frontend tests

* Minor

* Fix access of the frontend to the test nginx container

* Ensure to revert config change on exit

* Duplicate frontend config instead of temporarily changing it

* Fix set test compose name out of loop

* Fix redis connection

* Allow empty entrypoint -> default entrypoint is ignored

* Minor test improvement

* Add attribute "clear-entrypoint"

* Revert "Add attribute "clear-entrypoint""

This reverts commit 001b3ec.

* Remove test entrypoint with single word (not relevant for practice)

* Replace usage of 'yes' with 'tail' in test case

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Arne Tarara <[email protected]>
ArneTR added a commit that referenced this pull request Mar 16, 2025
* Add support for entrypoint attribute

* Fix entrypoint implementation

* Improve test case

* Add test case with script as entrypoint

* Add test case for entrypoint in conjunction with command

* Bump deepdiff from 8.2.0 to 8.3.0 (#1087)

Bumps [deepdiff](https://github.com/seperman/deepdiff) from 8.2.0 to 8.3.0.
- [Release notes](https://github.com/seperman/deepdiff/releases)
- [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst)
- [Commits](seperman/deepdiff@8.2.0...8.3.0)

---
updated-dependencies:
- dependency-name: deepdiff
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* E-Mail now made optional

* Checking int fields for params and limiting retrieval amount (#1089)

* Add compose stack name + allow test stack to run in parallel (#1093)

* Add compose stack name

* Allow test stack to run in parallel

* Fix frontend tests

* Minor

* Fix access of the frontend to the test nginx container

* Ensure to revert config change on exit

* Duplicate frontend config instead of temporarily changing it

* Fix set test compose name out of loop

* Fix redis connection

* Allow empty entrypoint -> default entrypoint is ignored

* Minor test improvement

* Add attribute "clear-entrypoint"

* Revert "Add attribute "clear-entrypoint""

This reverts commit 001b3ec.

* Remove test entrypoint with single word (not relevant for practice)

* Replace usage of 'yes' with 'tail' in test case

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Arne Tarara <[email protected]>
ArneTR added a commit that referenced this pull request Mar 22, 2025
* (Frontend): Energy-ID is now modular frontend component; Home page draft; Refactoring of config.js [skip ci]

* Added sample display of cards for connected tools [skip ci]

* h1 header change repositories / last 50 runs

* Normalized namings; Added /insigts endpoint

* Changing name to MetricRunner

* Updated meta

* Reverted metric-runner URL naming

* Modularized install script incl. help; Segmented out MetricRunner API

* Making metric runner main item also link to runs

* REplace CarbonDB and PowerHOG always

* More uniform style

* Bugfix for import

* Update README.md

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* Removed duplicate statement

* Updated EE

* Bump psycopg[binary] from 3.2.5 to 3.2.6 (#1095)

Bumps [psycopg[binary]](https://github.com/psycopg/psycopg) from 3.2.5 to 3.2.6.
- [Changelog](https://github.com/psycopg/psycopg/blob/master/docs/news.rst)
- [Commits](psycopg/psycopg@3.2.5...3.2.6)

---
updated-dependencies:
- dependency-name: psycopg[binary]
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Moving to pull_request_target to have secrets available even in external contributed PRs

* Add compose stack name + allow test stack to run in parallel (#1093)

* Add compose stack name

* Allow test stack to run in parallel

* Fix frontend tests

* Minor

* Fix access of the frontend to the test nginx container

* Ensure to revert config change on exit

* Duplicate frontend config instead of temporarily changing it

* Fix set test compose name out of loop

* Fix redis connection

* Add support for entrypoint attribute (#1088)

* Add support for entrypoint attribute

* Fix entrypoint implementation

* Improve test case

* Add test case with script as entrypoint

* Add test case for entrypoint in conjunction with command

* Bump deepdiff from 8.2.0 to 8.3.0 (#1087)

Bumps [deepdiff](https://github.com/seperman/deepdiff) from 8.2.0 to 8.3.0.
- [Release notes](https://github.com/seperman/deepdiff/releases)
- [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst)
- [Commits](seperman/deepdiff@8.2.0...8.3.0)

---
updated-dependencies:
- dependency-name: deepdiff
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* E-Mail now made optional

* Checking int fields for params and limiting retrieval amount (#1089)

* Add compose stack name + allow test stack to run in parallel (#1093)

* Add compose stack name

* Allow test stack to run in parallel

* Fix frontend tests

* Minor

* Fix access of the frontend to the test nginx container

* Ensure to revert config change on exit

* Duplicate frontend config instead of temporarily changing it

* Fix set test compose name out of loop

* Fix redis connection

* Allow empty entrypoint -> default entrypoint is ignored

* Minor test improvement

* Add attribute "clear-entrypoint"

* Revert "Add attribute "clear-entrypoint""

This reverts commit 001b3ec.

* Remove test entrypoint with single word (not relevant for practice)

* Replace usage of 'yes' with 'tail' in test case

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Arne Tarara <[email protected]>

* Unified display of home page blocks

* Typos and hardening

* Updated EE

* Test-Fix

* Renamed MetricRunner to ScenarioRunner

* Activate scenario runner not on every input, but only empty and Y/y

* Typo

* Removed additional info

* Bugfix: Inverted logic

* File renames

* Added insights tests

* Test fix

* Test install routine activates components always. No matter what other local config is

* Removing file from repo

* Typo

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: David Kopp <[email protected]>
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