Skip to content

refactor: several negative positioning/margin hacks #3775

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.

Sign up for GitHub

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
kaloudis merged 2 commits into from
Mar 2, 2026
Merged

refactor: several negative positioning/margin hacks #3775

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1754f2d
fix several negative positioning/margin hacks
myxmaster Mar 1, 2026
c76d003
refactor StyleSheet
myxmaster Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions views/AddNotes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,14 @@ export default class AddNotes extends React.Component<
multiline
numberOfLines={0}
style={{
padding: 20,
paddingHorizontal: 20,
flexGrow: 1,
flexShrink: 1,
backgroundColor: 'none'
}}
textInputStyle={{
height: '100%',
textAlignVertical: 'top',
marginTop: -13
textAlignVertical: 'top'
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 1, 2026

Choose a reason for hiding this comment

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

security-high high

The AddNotes component uses noteKey from route.params (line 41) as a direct key for storage operations (Storage.getItem, Storage.setItem). Since route parameters can be influenced via deep links, an attacker could potentially trick a user into overwriting critical application data (e.g., settings or contacts) by providing a malicious noteKey. For example, a link like zeusln://AddNotes?noteKey=zeus-settings-v2 would allow a user to inadvertently overwrite their entire application configuration if they save a note.

Recommendation: Validate that noteKey belongs to an expected set of keys or implement a restricted namespace for notes in storage to prevent unauthorized access to other application data.

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 Mar 2, 2026

Choose a reason for hiding this comment

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

unrelated to the PR

}}
value={notes}
placeholder={
Expand Down
106 changes: 57 additions & 49 deletions views/ContactDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,40 @@ export default class ContactDetails extends React.Component<
);
};

const avatarCircle = contact.photo ? (
<Image
source={{ uri: contact.getPhoto }}
style={{ width: 150, height: 150, borderRadius: 75 }}
/>
) : (
<View
style={{
width: 150,
height: 150,
borderRadius: 75,
backgroundColor: themeColor('secondary'),
alignItems: 'center',
justifyContent: 'center'
}}
>
{contact.getAvatarInitials ? (
<Text
style={{
fontSize: 48,
fontWeight: 'bold',
color: themeColor('text')
}}
>
{contact.getAvatarInitials}
</Text>
) : contact.hasOnlyCashuPubkey ? (
<Ecash fill="#FACC15" width={64} height={64} />
) : (
<Text style={{ fontSize: 64 }}>⚡</Text>
)}
</View>
);
Comment on lines +350 to +378
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 1, 2026

Choose a reason for hiding this comment

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

medium

While refactoring to create the avatarCircle component is a great improvement for code deduplication, the component still uses inline styles. To improve maintainability and adhere to best practices, consider moving these styles to the StyleSheet at the bottom of the file. This makes styles reusable and keeps the component's render method cleaner.

Here's a suggestion for the styles to add to your StyleSheet:

const styles = StyleSheet.create({
    // ... other styles
    avatarContainer: {
        width: 150,
        height: 150,
        borderRadius: 75,
    },
    avatarView: {
        backgroundColor: themeColor('secondary'),
        alignItems: 'center',
        justifyContent: 'center',
    },
    avatarInitials: {
        fontSize: 48,
        fontWeight: 'bold',
        color: themeColor('text'),
    },
    avatarIcon: {
        fontSize: 64,
    }
});

You can then apply these styles as shown in the code suggestion.

const avatarCircle = contact.photo ? (
    <Image
        source={{ uri: contact.getPhoto }}
        style={styles.avatarContainer}
    />
) : (
    <View style={[styles.avatarContainer, styles.avatarView]}>
        {contact.getAvatarInitials ? (
            <Text style={styles.avatarInitials}>
                {contact.getAvatarInitials}
            </Text>
        ) : contact.hasOnlyCashuPubkey ? (
            <Ecash fill="#FACC15" width={64} height={64} />
        ) : (
            <Text style={styles.avatarIcon}>⚡</Text>
        )}
    </View>
);

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 Mar 2, 2026

Choose a reason for hiding this comment

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

@myxmaster we can do this

Copy link
Contributor Author

@myxmaster myxmaster Mar 2, 2026

Choose a reason for hiding this comment

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

done (only moved those with more than 1 style, otherwise it is kind of pointless imo; also removed unused styles)


return (
<>
{isLoading ? (
Expand Down Expand Up @@ -388,60 +422,34 @@ export default class ContactDetails extends React.Component<
paddingBottom: 10
}}
>
{contact.banner && (
<Image
source={{ uri: contact.getBanner }}
style={{
width: '100%',
height: 150,
marginBottom: 20
}}
/>
)}
{contact.photo ? (
<Image
source={{ uri: contact.getPhoto }}
style={{
width: 150,
height: 150,
borderRadius: 75,
marginBottom: 20,
marginTop: contact.banner ? -100 : 0
}}
/>
) : (
{contact.banner ? (
<View
style={{
width: 150,
width: '100%',
height: 150,
borderRadius: 75,
marginBottom: 20,
marginTop: contact.banner ? -100 : 0,
backgroundColor:
themeColor('secondary'),
alignItems: 'center',
justifyContent: 'center'
marginBottom: 95
}}
>
{contact.getAvatarInitials ? (
<Text
style={{
fontSize: 48,
fontWeight: 'bold',
color: themeColor('text')
}}
>
{contact.getAvatarInitials}
</Text>
) : contact.hasOnlyCashuPubkey ? (
<Ecash
fill="#FACC15"
width={64}
height={64}
/>
) : (
<Text style={{ fontSize: 64 }}>⚡</Text>
)}
<Image
source={{ uri: contact.getBanner }}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 1, 2026

Choose a reason for hiding this comment

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

security-medium medium

The Image component's source.uri is populated from contact.getBanner, which is derived from untrusted contact data (e.g., from Nostr). The implementation of getBanner in models/Contact.ts (line 181) constructs a file:// URI by appending a user-controlled string to the app's document directory without sanitizing for path traversal sequences (..). This could allow a malicious contact to point the banner to sensitive internal files (e.g., rnfs://../../../../etc/hosts). While the impact is limited as the file is treated as an image, it still constitutes a path traversal vulnerability.

Recommendation: Sanitize the input in models/Contact.ts to prevent path traversal and validate that the resulting URI points to an expected location.

style={{
width: '100%',
height: 150
}}
/>
<View
style={{
position: 'absolute',
top: 75,
alignSelf: 'center'
}}
>
{avatarCircle}
</View>
</View>
) : (
<View style={{ marginBottom: 20 }}>
{avatarCircle}
</View>
)}
<Text
Expand Down
41 changes: 17 additions & 24 deletions views/Tools/Accounts/ImportAccount.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
View
} from 'react-native';
import { SafeAreaView } from 'react-native-safe-area-context';
import { ListItem } from '@rneui/themed';
import { inject, observer } from 'mobx-react';
import { Route } from '@react-navigation/native';
import { StackNavigationProp } from '@react-navigation/stack';
Expand Down Expand Up @@ -334,40 +333,34 @@ export default class ImportAccount extends React.Component<
}}
values={AddressTypes}
/>
<ListItem
containerStyle={{
borderBottomWidth: 0,
backgroundColor: 'transparent'
<View
style={{
flexDirection: 'row',
alignItems: 'center',
marginVertical: 20
}}
>
<ListItem.Title
<Text
style={{
color: themeColor('secondaryText'),
fontFamily: 'PPNeueMontreal-Book',
left: -10
fontSize: 17,
flex: 1
}}
>
{localeString(
'views.ImportAccount.existingAccount'
)}
</ListItem.Title>
<View
style={{
flex: 1,
flexDirection: 'row',
justifyContent: 'flex-end'
</Text>
<Switch
value={existing_account}
onValueChange={(value: boolean) => {
this.setState({
existing_account: value
});
}}
>
<Switch
value={existing_account}
onValueChange={(value: boolean) => {
this.setState({
existing_account: value
});
}}
/>
</View>
</ListItem>
/>
</View>
{existing_account && (
<>
<Text
Expand Down