Skip to content

fix: fix inconsistent naming pattern#2945

Open
JisoLya wants to merge 7 commits into
apache:masterfrom
JisoLya:arthas-sec
Open

fix: fix inconsistent naming pattern#2945
JisoLya wants to merge 7 commits into
apache:masterfrom
JisoLya:arthas-sec

Conversation

@JisoLya

@JisoLya JisoLya commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

Currently, the configuration keys in rest-server.properties use snake_case (e.g., server_port), which is inconsistent with the naming convention expected by ServerOptions.java. This mismatch causes the following issues:

  • User-defined configurations are ignored at startup.
  • The server defaults to hardcoded values in ServerOptions.java.

Terminal logs show warnings such as: "arthas.xxxx is redundant ...", indicating that the properties are not being recognized or registered.
image
image
image

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. store Store module labels Jan 27, 2026
@codecov

codecov Bot commented Jan 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.96%. Comparing base (03e6b8e) to head (4f3bfab).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2945      +/-   ##
============================================
- Coverage     36.80%   31.96%   -4.85%     
- Complexity      338      500     +162     
============================================
  Files           805      816      +11     
  Lines         68587    69738    +1151     
  Branches       9029     9226     +197     
============================================
- Hits          25241    22289    -2952     
- Misses        40653    44972    +4319     
+ Partials       2693     2477     -216     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 enhances security for Arthas debugging endpoints by restricting remote access and standardizing configuration naming patterns across the codebase.

Changes:

  • Added localhost-only access restriction to the store node's arthasstart endpoint
  • Standardized Arthas configuration property names from snake_case to camelCase (e.g., arthas.telnet_portarthas.telnetPort)
  • Changed default Arthas IP binding from 0.0.0.0 to 127.0.0.1 and expanded disabled commands to include jad,ognl,vmtool

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
hugegraph-store/hg-store-node/src/main/resources/application.yml Added Arthas configuration with localhost-only IP binding and expanded disabled commands
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java Added remote access check to arthasstart endpoint and new forbiddenMap helper method
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/AppConfig.java Updated default values for Arthas IP and disabled commands
hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties Renamed Arthas properties to camelCase and updated default values
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java Updated default values for Arthas IP binding and disabled commands

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.


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

@JisoLya JisoLya changed the title sec(store, server): disable remote access for arthasstart and fix inconsistent naming pattern fix: fix inconsistent naming pattern Jan 29, 2026
@JisoLya JisoLya requested a review from imbajin January 30, 2026 08:02
@github-actions

github-actions Bot commented Mar 1, 2026

Copy link
Copy Markdown

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

@VGalaxies VGalaxies left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review summary

  • Blocking: yes
  • Summary: A success path can now throw while constructing the response because nullable payloads are passed into Map.of().
  • Evidence:
    • Static review of git diff origin/master...HEAD and PartitionAPI.ok(...).

@JisoLya JisoLya requested review from VGalaxies and imbajin July 6, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inactive size:M This PR changes 30-99 lines, ignoring generated files. store Store module

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

4 participants