Skip to content

Commit

Permalink
chore: Cherry picked DIA-820 and LEAP-396 into release 1.11.0 (#5332)
Browse files Browse the repository at this point in the history
* chore: Cherry picked DIA-820 into release 1.11.0

* fix: LEAP-396: More exhaustative IP validation for SSRF defenses, plus user configurability (#5316)

* fix: LEAP-396: More exhaustative IP validation for SSRF, plus user configurability

* unbotch

* fix test

* more testing

* fix false positive finding from Bandit

* ci: Build frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7618278765

* [submodules] Copy src HumanSignal/dm2

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7618518612

* [submodules] Copy src HumanSignal/dm2

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7618540291

* ci: Build frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7618557170

* fix submodules

* fix submodules

---------

Co-authored-by: robot-ci-heartex <87703623+robot-ci-heartex@users.noreply.github.com>
Co-authored-by: Jo Booth <jo.m.booth@gmail.com>
Co-authored-by: robot-ci-heartex <robot-ci-heartex@users.noreply.github.com>
Co-authored-by: yyassi-heartex <104568407+yyassi-heartex@users.noreply.github.com>
  • Loading branch information
5 people committed Jan 22, 2024
1 parent 6a5f691 commit 2002e96
Show file tree
Hide file tree
Showing 21 changed files with 177 additions and 41 deletions.
2 changes: 2 additions & 0 deletions label_studio/core/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@
MAX_TIME_BETWEEN_ACTIVITY = int(get_env('MAX_TIME_BETWEEN_ACTIVITY', timedelta(days=5).total_seconds()))

SSRF_PROTECTION_ENABLED = get_bool_env('SSRF_PROTECTION_ENABLED', False)
USE_DEFAULT_BANNED_SUBNETS = get_bool_env('USE_DEFAULT_BANNED_SUBNETS', True)
USER_ADDITIONAL_BANNED_SUBNETS = get_env_list('USER_ADDITIONAL_BANNED_SUBNETS', default=[])

# user media files
MEDIA_ROOT = os.path.join(BASE_DATA_DIR, 'media')
Expand Down
57 changes: 47 additions & 10 deletions label_studio/core/utils/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,24 +197,61 @@ def validate_upload_url(url, block_local_urls=True):


def validate_ip(ip: str) -> None:
"""Checks if an IP is local/private.
"""If settings.USE_DEFAULT_BANNED_SUBNETS is True, this function checks
if an IP is reserved for any of the reasons in
https://en.wikipedia.org/wiki/Reserved_IP_addresses
and raises an exception if so. Additionally, if settings.USER_ADDITIONAL_BANNED_SUBNETS
is set, it will also check against those subnets.
If settings.USE_DEFAULT_BANNED_SUBNETS is False, this function will only check
the IP against settings.USER_ADDITIONAL_BANNED_SUBNETS. Turning off the default
subnets is **risky** and should only be done if you know what you're doing.
:param ip: IP address to be checked.
"""

if ip == '0.0.0.0': # nosec
raise InvalidUploadUrlError
default_banned_subnets = [
'0.0.0.0/8', # current network
'10.0.0.0/8', # private network
'100.64.0.0/10', # shared address space
'127.0.0.0/8', # loopback
'169.254.0.0/16', # link-local
'172.16.0.0/12', # private network
'192.0.0.0/24', # IETF protocol assignments
'192.0.2.0/24', # TEST-NET-1
'192.88.99.0/24', # Reserved, formerly ipv6 to ipv4 relay
'192.168.0.0/16', # private network
'198.18.0.0/15', # network interconnect device benchmark testing
'198.51.100.0/24', # TEST-NET-2
'203.0.113.0/24', # TEST-NET-3
'224.0.0.0/4', # multicast
'233.252.0.0/24', # MCAST-TEST-NET
'240.0.0.0/4', # reserved for future use
'255.255.255.255/32', # limited broadcast
'::/128', # unspecified address
'::1/128', # loopback
'::ffff:0:0/96', # IPv4-mapped address
'::ffff:0:0:0/96', # IPv4-translated address
'64:ff9b::/96', # IPv4/IPv6 translation
'64:ff9b:1::/48', # IPv4/IPv6 translation
'100::/64', # discard prefix
'2001:0000::/32', # Teredo tunneling
'2001:20::/28', # ORCHIDv2
'2001:db8::/32', # documentation
'2002::/16', # 6to4
'fc00::/7', # unique local
'fe80::/10', # link-local
'ff00::/8', # multicast
]

local_subnets = [
'127.0.0.0/8',
'10.0.0.0/8',
'172.16.0.0/12',
'192.168.0.0/16',
banned_subnets = [
*(default_banned_subnets if settings.USE_DEFAULT_BANNED_SUBNETS else []),
*(settings.USER_ADDITIONAL_BANNED_SUBNETS or []),
]

for subnet in local_subnets:
for subnet in banned_subnets:
if ipaddress.ip_address(ip) in ipaddress.ip_network(subnet):
raise InvalidUploadUrlError
raise InvalidUploadUrlError(f'URL resolves to a reserved network address (block: {subnet})')


def ssrf_safe_get(url, *args, **kwargs):
Expand Down
2 changes: 1 addition & 1 deletion label_studio/frontend/dist/dm/js/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion label_studio/frontend/dist/dm/js/main.js.map

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions label_studio/frontend/dist/dm/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "feat: OPTIC-107: Draft saving on view all",
"commit": "b2006083fb40a3d5a1ee7979ff24497864dfcdac",
"branch": "master",
"message": "chore: Cherry picked DIA-820 into release 1.11.0",
"commit": "02f190d464e3236c17217e3089ee1892aa5919d9",
"branch": "ls-release/1.11.0 ",
"date": "2024-01-04T09:37:55Z"
}
}
49 changes: 48 additions & 1 deletion label_studio/tests/data_import/test_uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,54 @@ def test_local_url_after_redirect(self, project, settings):
ValidationError
) as e:
load_tasks(request, project)
assert 'The provided URL was not valid.' in str(e.value)
assert 'URL resolves to a reserved network address (block: 127.0.0.0/8)' in str(e.value)

def test_user_specified_block(self, project, settings):
settings.SSRF_PROTECTION_ENABLED = True
settings.USER_ADDITIONAL_BANNED_SUBNETS = ['1.2.3.4']
request = MockedRequest(url='http://validurl.com')

# Mock the necessary parts of the response object
mock_response = Mock()
mock_response.raw._connection.sock.getpeername.return_value = ('1.2.3.4', 8080)

# Patch the requests.get call in the data_import.uploader module
with mock.patch('core.utils.io.requests.get', return_value=mock_response), pytest.raises(
ValidationError
) as e:
load_tasks(request, project)
assert 'URL resolves to a reserved network address (block: 1.2.3.4)' in str(e.value)

mock_response.raw._connection.sock.getpeername.return_value = ('198.51.100.0', 8080)
with mock.patch('core.utils.io.requests.get', return_value=mock_response), pytest.raises(
ValidationError
) as e:
load_tasks(request, project)
assert 'URL resolves to a reserved network address (block: 198.51.100.0/24)' in str(e.value)

def test_user_specified_block_without_default(self, project, settings):
settings.SSRF_PROTECTION_ENABLED = True
settings.USER_ADDITIONAL_BANNED_SUBNETS = ['1.2.3.4']
settings.USE_DEFAULT_BANNED_SUBNETS = False
request = MockedRequest(url='http://validurl.com')

# Mock the necessary parts of the response object
mock_response = Mock()
mock_response.raw._connection.sock.getpeername.return_value = ('1.2.3.4', 8080)

# Patch the requests.get call in the data_import.uploader module
with mock.patch('core.utils.io.requests.get', return_value=mock_response), pytest.raises(
ValidationError
) as e:
load_tasks(request, project)
assert 'URL resolves to a reserved network address (block: 1.2.3.4)' in str(e.value)

mock_response.raw._connection.sock.getpeername.return_value = ('198.51.100.0', 8080)
with mock.patch('core.utils.io.requests.get', return_value=mock_response), pytest.raises(
ValidationError
) as e:
load_tasks(request, project)
assert "'Mock' object is not subscriptable" in str(e.value) # validate ip did not raise exception


class TestTasksFileChecks:
Expand Down
2 changes: 1 addition & 1 deletion web/dist/apps/labelstudio/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/apps/labelstudio/main.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/datamanager/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/datamanager/main.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/editor/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/editor/main.js.map

Large diffs are not rendered by default.

25 changes: 25 additions & 0 deletions web/libs/datamanager/src/components/CellViews/ProjectCell.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { getRoot } from "mobx-state-tree";

export const ProjectCell = (cell) => {
const { original, value } = cell;
const root = getRoot(original);
// TODO: turn this into a link to open the project later
const projectList = value
.map((projectRef) => (
root.taskStore.associatedList.find(proj => proj.id === projectRef.project_id)?.title
))
.filter(Boolean);

return (
<div
style={{
maxHeight: "100%",
overflow: "hidden",
fontSize: 12,
lineHeight: "16px",
}}
>
{projectList && projectList.join(", ")}
</div>
);
};
2 changes: 2 additions & 0 deletions web/libs/datamanager/src/components/CellViews/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ export { NumberCell as Number } from "./NumberCell";
export { StringCell as String } from "./StringCell";
export { StringCell as Text } from "./StringCell";
export { VideoCell as Video } from "./VideoCell";
export { ProjectCell as Project } from './ProjectCell';

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { TableContext, TableElem } from "../TableContext";
import { getStyle } from "../utils";
import "./TableHead.styl";
import { FF_DEV_2984, FF_DEV_3873, FF_LOPS_E_10, isFF } from "../../../../utils/feature-flags";
import { getRoot } from "mobx-state-tree";

const { Block, Elem } = BemWithSpecifiContext();

Expand Down Expand Up @@ -120,9 +121,11 @@ const ColumnRenderer = observer(
);
}

const root = getRoot(column.original);
const isDE = root.SDK.type === "DE";
const canOrder = sortingEnabled && column.original?.canOrder;
const Decoration = decoration?.get?.(column);
const extra = columnHeaderExtra
const extra = !isDE && columnHeaderExtra
? columnHeaderExtra(column, Decoration)
: null;
const content = Decoration?.content
Expand Down Expand Up @@ -158,7 +161,7 @@ const ColumnRenderer = observer(
onResizeFinished={(width) => onResize?.(column, width)}
onReset={() => onReset?.(column)}
>
{column.parent ? (
{!isDE && column.parent ? (
<DropdownWrapper
column={column}
cellViews={cellViews}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { TableContext, TableElem } from "../TableContext";
import { getStyle } from "../utils";
import "./TableHead.styl";
import { FF_DEV_2984, FF_DEV_3873, isFF } from "../../../../utils/feature-flags";
import { getRoot } from "mobx-state-tree";

const { Block, Elem } = BemWithSpecifiContext();

Expand Down Expand Up @@ -110,9 +111,11 @@ const ColumnRenderer = observer(
);
}

const root = getRoot(column.original);
const isDE = root.SDK.type === "DE";
const canOrder = sortingEnabled && column.original?.canOrder;
const Decoration = decoration?.get?.(column);
const extra = columnHeaderExtra
const extra = !isDE && columnHeaderExtra
? columnHeaderExtra(column, Decoration)
: null;
const content = Decoration?.content
Expand Down Expand Up @@ -148,7 +151,7 @@ const ColumnRenderer = observer(
onResizeFinished={(width) => onResize?.(column, width)}
onReset={() => onReset?.(column)}
>
{column.parent ? (
{!isDE && column.parent ? (
<DropdownWrapper
column={column}
cellViews={cellViews}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export const ActionsButton = injector(observer(({ store, size, hasSelected, ...r
const submenuRef = useRef();
const onClick = useCallback((e) => {
e.preventDefault();
e.stopPropagation();
if (action.disabled) return;
action?.callback ? action?.callback(store.currentView?.selected?.snapshot, action) : invokeAction(action, isDeleteAction);
parentRef?.current?.close?.();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,13 @@ export const DataView = injector(
},
},
{
resolver: (col) => col.type === "Image",
resolver: (col) => col.type === "Image" && col.original && getRoot(col.original)?.SDK?.type !== 'DE',
style: { width: 150, justifyContent: "center" },
},
{
resolver: (col) => col.type === "Image" && col.original && getRoot(col.original)?.SDK?.type === 'DE',
style: { width: 150 },
},
{
resolver: (col) => ["Date", "Datetime"].includes(col.type),
style: { width: 240 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const DataView = injector(
} else if (e.metaKey || e.ctrlKey) {
window.open(`./?task=${itemID}`, "_blank");
} else {
if (isFF(FF_OPTIC_2)) await store._sdk.lsf?.saveDraft();
if (isFF(FF_OPTIC_2)) store._sdk.lsf?.saveDraft();
getRoot(view).startLabeling(item);
}
},
Expand Down Expand Up @@ -247,9 +247,13 @@ export const DataView = injector(
},
},
{
resolver: (col) => col.type === "Image",
resolver: (col) => col.type === "Image" && col.original && getRoot(col.original)?.SDK?.type !== 'DE',
style: { width: 150, justifyContent: "center" },
},
{
resolver: (col) => col.type === "Image" && col.original && getRoot(col.original)?.SDK?.type === 'DE',
style: { width: 150 },
},
{
resolver: (col) => ["Date", "Datetime"].includes(col.type),
style: { width: 240 },
Expand Down
26 changes: 18 additions & 8 deletions web/libs/datamanager/src/sdk/lsf-sdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -677,42 +677,50 @@ export class LSFWrapper {
};

draftToast = (status) => {

if (status === 200 || status === 201) this.datamanager.invoke("toast", { message: "Draft saved successfully", type: "info" });
else if (status !== undefined) this.datamanager.invoke("toast", { message: "There was an error saving your draft", type: "error" });
}

needsDraftSave = (annotation) => {
if (annotation.history?.hasChanges && !annotation.draftSaved) return true;
if (annotation.history?.hasChanges && new Date(annotation.history.lastAdditionTime) > new Date(annotation.draftSaved)) return true;
return false;
}

saveDraft = async (target = null) => {
const selected = target || this.lsf?.annotationStore?.selected;
const hasChanges = selected.history.hasChanges;
const submissionInProgress = selected?.submissionStarted;
const draftIsFresh = new Date(selected.draftSaved) > new Date() - selected.autosaveDelay;
const hasChanges = this.needsDraftSave(selected);

if (selected?.isDraftSaving || draftIsFresh) {
if (selected?.isDraftSaving) {
await when(() => !selected.isDraftSaving);
this.draftToast(200);
}
else if (hasChanges && selected && !submissionInProgress) {
else if (hasChanges && selected) {
const res = await selected?.saveDraftImmediatelyWithResults();
const status = res?.$meta?.status;

this.draftToast(status);
}
};
};

onSubmitDraft = async (studio, annotation, params = {}) => {
const annotationDoesntExist = !annotation.pk;
const data = { body: this.prepareData(annotation, { draft: true }) }; // serializedAnnotation
const hasChanges = this.needsDraftSave(annotation);
const showToast = params?.useToast && hasChanges;
// console.log('onSubmitDraft', params?.useToast, hasChanges);

if (params?.useToast) delete params.useToast;

Object.assign(data.body, params);

await this.saveUserLabels();

if (annotation.draftId > 0) {
// draft has been already created
const res = await this.datamanager.apiCall("updateDraft", { draftID: annotation.draftId }, data);


showToast && this.draftToast(res?.$meta?.status);
return res;

} else {
Expand All @@ -728,6 +736,8 @@ export class LSFWrapper {
);
}
response?.id && annotation.setDraftId(response?.id);
showToast && this.draftToast(response?.$meta?.status);

return response;
}
};
Expand Down
4 changes: 2 additions & 2 deletions web/libs/datamanager/src/utils/bem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ const assembleClass = (block: string, elem?: string, mix?: CNMix | CNMix[], mod?
}

const attachNamespace = (cls: string) => {
if (new RegExp(CSS_PREFIX).test(cls)) return cls;
else return `${CSS_PREFIX}${cls}`;
if (typeof cls !== 'string') console.error('Non-string classname: ', cls);
return String(cls).startsWith(CSS_PREFIX) ? cls : `${CSS_PREFIX}${cls}`;
};

return finalClass.map(attachNamespace).join(" ");
Expand Down

0 comments on commit 2002e96

Please sign in to comment.