-
Notifications
You must be signed in to change notification settings - Fork 331
Security: Hardcoded Tika Server URL without Authentication #2275
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,13 +15,14 @@ | |||||||||||||||
| # limitations under the License. | ||||||||||||||||
|
|
||||||||||||||||
| import io | ||||||||||||||||
| import os | ||||||||||||||||
| from typing import Dict, Any, Optional, List | ||||||||||||||||
|
|
||||||||||||||||
| import pandas as pd | ||||||||||||||||
|
|
||||||||||||||||
| import requests | ||||||||||||||||
|
|
||||||||||||||||
| TIKA_URL = "http://tika:9998/tika" | ||||||||||||||||
| TIKA_URL = os.environ.get("TIKA_URL", "http://tika:9998/tika") | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def tika_extractor( | ||||||||||||||||
|
Comment on lines
+25
to
28
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/common/api/internal/extract/pdf/engines/tika.py
Line: 25-28
Comment:
`TIKA_URL` is resolved once at module import time. If `TIKA_URL` is set in the environment *after* the module is imported (a common pattern in test suites or dynamic container environments), the change will be silently ignored and the stale value will be used for every subsequent call. Moving the lookup inside the function (or using `functools.lru_cache` for efficiency) makes the value reflect the environment at the time of each call.
```suggestion
_DEFAULT_TIKA_URL = "http://tika:9998/tika"
def tika_extractor(
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||
|
|
||||||||||||||||
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.
The
test-coverage-new-codeproject rule requires that new or modified business logic includes corresponding unit tests. This change introduces a runtime-configurable code path (env var override ofTIKA_URL) with no accompanying tests. A test verifying thatos.environ["TIKA_URL"]is picked up correctly (and that the default is used when unset) would guard against regressions where the lookup is accidentally removed or overridden.Rule Used: New functionality must include corresponding unit ... (source)
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!